Re: GSoC: PATH_MAX

[Patrik Olsson]

Hello,

On 07/04/11 21:42, olafBuddenhagen@gmx.net wrote:
> 
> 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.
> 

I am aware of that, but from what I could gather, that package was very
GNU/Linux-specific. In fact, I was not even sure if it would be useful
on Hurd at all (I assumed it was since it was being built for hurd-i386
after all).

>> Note that they are currently untested.
> 
> Hope you can test them soon :-)
> 

I will probably this weekend. :-)

>> 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.)
> 

I know it depends on the situation, I just didn't know what applied in
those particular situations I had. :-) I assumed this was a pretty
low-level library so I found that propagating the error should be the
most correct. However, for the snprintf calls this was not done so I
decided not to do it for the asprintf calls either.

>> +    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.
> 

The reason I added the check is because the original code does it too
for other variables in the previous lines, and I preferred to be consistent.

And it could also have one advantage. If one wants each alloc to match a
free during the course of a program execution, then the check should be
there. I'm not sure how useful it would be though, it's just a thought.

/Patrik

signature.asc
Description: OpenPGP digital signature