[56] in bug-owl
Re: Instances with commas break M-N
daemon@ATHENA.MIT.EDU (Mitchell E Berger)
Wed Jul 10 18:30:14 2002
Message-Id: <200207102230.SAA22647@athenaphobia.mit.edu>
To: bug-owl@MIT.EDU
cc: boojum@MIT.EDU, mitchb@MIT.EDU
In-reply-to: [0055] in bug-owl
Date: Wed, 10 Jul 2002 18:30:12 -0400
From: Mitchell E Berger <mitchb@MIT.EDU>
>If I receive a zephyr with an instance with a comma in it, and attempt
>to set a filter on class, instance with M-N, I first get an error
>message "Unknown filter"; after that, I often seem to get a
>Segmentation Violation upon doing the next thing; one reproducible way
>is if the offending zephyr is the last in the stack, do M-N and then
>n.
In a discussion with Laura, we discovered that the instance she had been using
actually had both a comma and a space in it (I couldn't reproduce it with a
comma), and that in fact it was the space alone which was screwing it up.
I looked around and found two major issues contributing to this behavior. One
is that the "smart" narrowing code wasn't quoting the regexps it forms for
classes and instances, which is why the filter didn't get created, and the
reason for the "Unknown filter" error message. The more troubling issue is
that the code to initialize the filter was in fact detecting illegal syntax
correctly and returning an error code for it - the narrowing code was ignoring
the return value, though, and adding the uninitialized filter to the list of
filters. This was causing the segfaults.
The following patch against current CVS does these things:
- Fixes Laura's specific bug
- Makes owl filter code much more paranoid about ensuring that if we get
errors, they are handled and reported properly, so even if we have another
similar bug (I suspect we may have one if classes/instances contain quotes,
but am not psyched enough to check right now), it will politely not make
the filter, report the error, not try switching to it, and not segfault
(or at least not because of this problem)
- Does some better checking of the result of mallocing things, and changes
some things to use owl_malloc instead of direct malloc for consistency
Mitch
Index: commands.c
===================================================================
RCS file: /afs/athena.mit.edu/astaff/project/ktools/src/owl/repository/owl/commands.c,v
retrieving revision 1.12
diff -u -p -r1.12 commands.c
--- commands.c 2002/07/09 02:37:22 1.12
+++ commands.c 2002/07/10 21:19:45
@@ -778,8 +778,10 @@ char *owl_command_smartnarrow(int argc,
}
if (filtname) {
owl_function_change_view(filtname);
- owl_free(filtname);
+ } else {
+ owl_function_makemsg("Internal error creating filter for smartnarrow");
}
+ owl_free(filtname);
return NULL;
}
@@ -1290,7 +1292,11 @@ char *owl_command_viewclass(int argc, ch
return NULL;
}
filtname = owl_function_fastclassinstfilt(argv[1], NULL);
- owl_function_change_view(filtname);
+ if (!filtname) {
+ owl_function_makemsg("Internal error creating filter for viewclass");
+ } else {
+ owl_function_change_view(filtname);
+ }
owl_free(filtname);
return NULL;
}
Index: functions.c
===================================================================
RCS file: /afs/athena.mit.edu/astaff/project/ktools/src/owl/repository/owl/functions.c,v
retrieving revision 1.12
diff -u -p -r1.12 functions.c
--- functions.c 2002/07/09 19:54:57 1.12
+++ functions.c 2002/07/10 22:12:53
@@ -1694,7 +1694,7 @@ char *owl_function_fastclassinstfilt(cha
owl_list *fl;
owl_filter *f;
char *argbuff, *filtname;
- int len;
+ int len, ret;
fl=owl_global_get_filterlist(&g);
@@ -1702,6 +1702,9 @@ char *owl_function_fastclassinstfilt(cha
len=strlen(class)+30;
if (instance) len+=strlen(instance);
filtname=owl_malloc(len);
+ if (!filtname) {
+ return NULL;
+ }
if (!instance) {
sprintf(filtname, "class-%s", class);
} else {
@@ -1716,24 +1719,40 @@ char *owl_function_fastclassinstfilt(cha
/* create the new filter */
argbuff=owl_malloc(len+20);
- sprintf(argbuff, "( class ^%s$ )", class);
+ if (!argbuff) {
+ owl_free(filtname);
+ return NULL;
+ }
+ sprintf(argbuff, "( class '^%s$' )", class);
if (instance) {
- sprintf(argbuff, "%s and ( instance ^%s$ )", argbuff, instance);
+ sprintf(argbuff, "%s and ( instance '^%s$' )", argbuff, instance);
}
f=owl_malloc(sizeof(owl_filter));
- owl_filter_init_fromstring(f, filtname, argbuff);
+ if (!f) {
+ owl_free(filtname);
+ owl_free(argbuff);
+ return NULL;
+ }
+ ret=owl_filter_init_fromstring(f, filtname, argbuff);
+ owl_free(argbuff);
+ if (ret==-1) {
+ owl_free(filtname);
+ owl_free(f);
+ return NULL;
+ }
+
/* add it to the global list */
owl_global_add_filter(&g, f);
- owl_free(argbuff);
return filtname;
}
char *owl_function_fastuserfilt(char *user) {
owl_filter *f;
char *argbuff, *longuser, *shortuser, *filtname;
+ int ret;
/* stick the local realm on if it's not there */
longuser=long_sender(user);
@@ -1741,6 +1760,11 @@ char *owl_function_fastuserfilt(char *us
/* name for the filter */
filtname=owl_malloc(strlen(shortuser)+20);
+ if (!filtname) {
+ owl_free(longuser);
+ owl_free(shortuser);
+ return NULL;
+ }
sprintf(filtname, "user-%s", shortuser);
/* if it already exists then go with it. This lets users override */
@@ -1750,31 +1774,54 @@ char *owl_function_fastuserfilt(char *us
/* create the new-internal filter */
f=owl_malloc(sizeof(owl_filter));
+ if (!f) {
+ owl_free(longuser);
+ owl_free(shortuser);
+ owl_free(filtname);
+ return NULL;
+ }
argbuff=owl_malloc(strlen(longuser)+200);
+ if (!argbuff) {
+ owl_free(longuser);
+ owl_free(shortuser);
+ owl_free(filtname);
+ owl_free(f);
+ return NULL;
+ }
sprintf(argbuff, "( ( class ^message$ ) and ( instance ^personal$ ) and ( sender ^%s$ ) )", longuser);
sprintf(argbuff, "%s or ( ( type ^admin$ ) and ( recipient %s ) )", argbuff, shortuser);
sprintf(argbuff, "%s or ( ( class ^login$ ) and ( sender ^%s$ ) )", argbuff, longuser);
-
- owl_filter_init_fromstring(f, filtname, argbuff);
- /* add it to the global list */
- owl_global_add_filter(&g, f);
+ ret=owl_filter_init_fromstring(f, filtname, argbuff);
/* free stuff */
- owl_free(argbuff);
owl_free(longuser);
owl_free(shortuser);
+ owl_free(argbuff);
+ if (ret==-1) {
+ owl_free(filtname);
+ owl_free(f);
+ return NULL;
+ }
+
+ /* add it to the global list */
+ owl_global_add_filter(&g, f);
+
return filtname;
}
char *owl_function_fasttypefilt(char *type) {
owl_filter *f;
char *argbuff, *filtname;
+ int ret;
/* name for the filter */
filtname=owl_sprintf("type-%s", type);
+ if (!filtname) {
+ return NULL;
+ }
/* if it already exists then go with it. This lets users override */
if (owl_global_get_filter(&g, filtname)) {
@@ -1783,17 +1830,32 @@ char *owl_function_fasttypefilt(char *ty
/* create the new-internal filter */
f=owl_malloc(sizeof(owl_filter));
+ if (!f) {
+ owl_free(filtname);
+ return NULL;
+ }
argbuff = owl_sprintf("type ^%s$", type);
-
- owl_filter_init_fromstring(f, filtname, argbuff);
+ if (!argbuff) {
+ owl_free(filtname);
+ owl_free(f);
+ return NULL;
+ }
- /* add it to the global list */
- owl_global_add_filter(&g, f);
+ ret=owl_filter_init_fromstring(f, filtname, argbuff);
/* free stuff */
owl_free(argbuff);
+ if (ret==-1) {
+ owl_free(filtname);
+ owl_free(f);
+ return NULL;
+ }
+
+ /* add it to the global list */
+ owl_global_add_filter(&g, f);
+
return filtname;
}
@@ -1974,8 +2036,12 @@ void owl_function_zpunt(char *class, cha
fl=owl_global_get_puntlist(&g);
/* first, create the filter */
- f=malloc(sizeof(owl_filter));
- buff=malloc(strlen(class)+strlen(inst)+strlen(recip)+100);
+ f=owl_malloc(sizeof(owl_filter));
+ buff=owl_malloc(strlen(class)+strlen(inst)+strlen(recip)+100);
+ if (!f || !buff) {
+ owl_function_makemsg("Out of memory creating filter for zpunt");
+ if (f) owl_free(f);
+ }
if (!strcmp(recip, "*")) {
sprintf(buff, "class ^%s$ and instance ^%s$", class, inst);
} else {
@@ -1984,7 +2050,7 @@ void owl_function_zpunt(char *class, cha
owl_function_debugmsg("About to filter %s", buff);
ret=owl_filter_init_fromstring(f, "punt-filter", buff);
owl_free(buff);
- if (ret) {
+ if (ret==-1) {
owl_function_makemsg("Error creating filter for zpunt");
owl_filter_free(f);
return;
Index: owl.c
===================================================================
RCS file: /afs/athena.mit.edu/astaff/project/ktools/src/owl/repository/owl/owl.c,v
retrieving revision 1.3
diff -u -p -r1.3 owl.c
--- owl.c 2002/06/30 20:58:14 1.3
+++ owl.c 2002/07/10 22:04:20
@@ -133,21 +133,49 @@ int main(int argc, char **argv, char **e
owl_context_set_readconfig(owl_global_get_context(&g));
/* setup the default filters */
- f=malloc(sizeof(owl_filter));
- owl_filter_init_fromstring(f, "personal", "( class ^message$ and instance ^personal$ ) or ( type ^admin$ and recipient .+ )"); /* fix to use admintype */
- owl_list_append_element(owl_global_get_filterlist(&g), f);
+ f=owl_malloc(sizeof(owl_filter));
+ if (!f) {
+ fprintf(stderr, "Out of memory setting up default personal filter.\n");
+ exit(1);
+ }
+ if (owl_filter_init_fromstring(f, "personal", "( class ^message$ and instance ^personal$ ) or ( type ^admin$ and recipient .+ )") == -1) { /* fix to use admintype */
+ fprintf(stderr, "Error initializing default personal filter.\n");
+ } else {
+ owl_list_append_element(owl_global_get_filterlist(&g), f);
+ }
- f=malloc(sizeof(owl_filter));
- owl_filter_init_fromstring(f, "trash", "class ^mail$ or opcode ^ping$ or type ^admin$ or class ^login$");
- owl_list_append_element(owl_global_get_filterlist(&g), f);
+ f=owl_malloc(sizeof(owl_filter));
+ if (!f) {
+ fprintf(stderr, "Out of memory setting up default trash filter.\n");
+ exit(1);
+ }
+ if (owl_filter_init_fromstring(f, "trash", "class ^mail$ or opcode ^ping$ or type ^admin$ or class ^login$") == -1) {
+ fprintf(stderr, "Error initializing default trash filter.\n");
+ } else {
+ owl_list_append_element(owl_global_get_filterlist(&g), f);
+ }
- f=malloc(sizeof(owl_filter));
- owl_filter_init_fromstring(f, "reply-lockout", "class ^noc or class ^mail$");
- owl_list_append_element(owl_global_get_filterlist(&g), f);
+ f=owl_malloc(sizeof(owl_filter));
+ if (!f) {
+ fprintf(stderr, "Out of memory setting up default reply-lockout filter.\n");
+ exit(1);
+ }
+ if (owl_filter_init_fromstring(f, "reply-lockout", "class ^noc or class ^mail$") == -1) {
+ fprintf(stderr, "Error initializing default reply-lockout filter.\n");
+ } else {
+ owl_list_append_element(owl_global_get_filterlist(&g), f);
+ }
- f=malloc(sizeof(owl_filter));
- owl_filter_init_fromstring(f, "all", "class .*");
- owl_list_append_element(owl_global_get_filterlist(&g), f);
+ f=owl_malloc(sizeof(owl_filter));
+ if (!f) {
+ fprintf(stderr, "Out of memory setting up default all filter.\n");
+ exit(1);
+ }
+ if (owl_filter_init_fromstring(f, "all", "class .*") == -1) {
+ fprintf(stderr, "Error initializing default all filter.\n");
+ } else {
+ owl_list_append_element(owl_global_get_filterlist(&g), f);
+ }
/* set the current view */
owl_view_create(owl_global_get_current_view(&g), f);
