Alessandro Vesely <[EMAIL PROTECTED]> writes:
> A few mumblings about that patch
>
> Lloyd Zusman wrote:
> [...]
>> +#define FILTER_LIST_INCREMENT 8
>> +
>> +static char **filterlist = NULL;
>> +static int filterlistsize = 0;
>> +static int nfilters = 0;
>> +
>> +static void free_filters()
>> +{
>> + if (filterlist != NULL)
>> + {
>> + for (int n = 0; n < nfilters; n++)
>> + {
>> + if (filterlist[n] != NULL)
>
> That test is completely bogus, I would remove it.
> Besides being useless because free repeats it anyway,
> it may fool an occasional reader of the source into
> thinking that filterlist entries should have been
> initialized to NULL.
The free() routine does not always check for NULL. There are some
systems on which it throws a segfault if NULL is passed to it. This may
not be the documented behavior, but I have seen it occur, nonetheless.
However, given the test for NULL after strdup() that you correctly
mention below, the NULL test before free() would indeed be superfluous
if that strdup() NULL test were in place.
>> + {
>> + free(filterlist[n]);
>> + }
>> + }
>> + }
>> + nfilters = 0;
>> +}
>> +
>> +static int add_filter(const char *filter)
>> +{
>> + if (nfilters >= filterlistsize)
>> + {
>> + if (filterlist == NULL)
>> + {
>> + filterlist = (char **) malloc(sizeof (char *) *
>> + FILTER_LIST_INCREMENT);
>> + }
>> + else
>> + {
>> + filterlist = (char **) realloc(filterlist,
>> + sizeof (char *) *
>> + (filterlistsize +
>> + FILTER_LIST_INCREMENT));
>> + }
>> + if (filterlist == NULL)
>> + {
>> + cout << "432 Out of memory when processing mail
>> filters.\n"
>> + << flush;
>> + return (1);
>> + }
>> + filterlistsize += FILTER_LIST_INCREMENT;
>> + }
>> + filterlist[nfilters++] = strdup(filter);
>> + return (0);
>> +}
>
> Hmmm, strdup might also fail... what about the following
>
> + if (nfilters >= filterlistsize)
> + {
> + if (filterlist == NULL)
> + {
> + filterlist = (char **) malloc(sizeof (char *) *
> + FILTER_LIST_INCREMENT);
> + }
> + else
> + {
> + filterlist = (char **) realloc(filterlist,
> + sizeof (char *) *
> + (filterlistsize +
> + FILTER_LIST_INCREMENT));
> + }
> + if (filterlist != NULL)
> + filterlistsize += FILTER_LIST_INCREMENT;
> + }
> + if (filterlist == NULL ||
> + (filterlist[nfilters+1] = strdup(filter)) == NULL)
> + {
> + cout << "432 Out of memory when processing mail filters.\n"
> + << flush;
> + return (1);
> + }
> + nfilters += 1;
> + return (0);
>
>> [...]
>> @@ -47,16 +97,29 @@
>> sockname = FILTERSOCKETDIR "/";
>> sockname += de->d_name;
>> - if (dofilter( sockname,
>> - filename, nmsgids,
>> - msgidfunc,
>> - funcarg))
>> + if (add_filter(sockname) != 0)
>> {
>> - closedir(dirp);
>
> why don't you closedir in case of memory failure?
>
>> return (1);
>> }
>> }
>> if (dirp) closedir(dirp);
>> +
>> + qsort((void *) filterlist,
>> + (size_t) nfilters,
>> + sizeof (char *),
>> + (int (*)(const void*, const void*)) strcmp);
>
>
> -------------------------------------------------------
> This SF.Net email is sponsored by xPML, a groundbreaking scripting language
> that extends applications into web and mobile media. Attend the live webcast
> and join the prime developer group breaking into this new coding territory!
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
> _______________________________________________
> courier-users mailing list
> [email protected]
> Unsubscribe: https://lists.sourceforge.net/lists/listinfo/courier-users
>
--
Lloyd Zusman
[EMAIL PROTECTED]
God bless you.
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
courier-users mailing list
[email protected]
Unsubscribe: https://lists.sourceforge.net/lists/listinfo/courier-users
