Hello, On 07/04/11 21:42, olafbuddenha...@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
