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-