Re: GSoC: PATH_MAX

[olafBuddenhagen]

Hi,

On Wed, Apr 06, 2011 at 07:02:14PM +0200, Patrik Olsson wrote:

> As a preparation exercise for GSoC, I have fixed the PATH_MAX issue in
> three Debian packages: nekobee, whysynth and libsepol.

Great. From what I can see, these are all pretty simple cases -- but
it's certainly a good start :-)

A general remark: not all plattforms have asprintf(). Depending on how
portable the packages in question are, you might need to add
configure-time checks and/or compile-time conditionals.

> Note that they are currently untested.

Hope you can test them soon :-)

> Also note that I did not take the opportunity to fix unrelated bugs,
> such as in the cases where the code does not check the return value of
> snprintf. In those cases, I also just ignore the return value of
> asprintf, since I don't know what would be the appropriate way to
> handle such error anyway.

This depends on the situation; but for most programs, the usual way to
handle allocation errors is bailing out, using error() or something
similar. (For low-level libraries it's sometimes preferable to propagate
the error code; but that's not always possible or useful.)

> +    if (patches_tmp_filename != NULL) +
> g_free(patches_tmp_filename);

I don't know how g_free() behaves -- but if it's the same as normal
free(), you don't need the conditional: free(NULL) is a no-op by
definition.

Glancing over your patches, I don't see any other obvious shortcoming
:-) However, I didn't check the context of the patches, so this is not a
full review.

-antrik-