[pacman-dev] [PATCH] Prevent warnings with unchecked asprintf and FORTIFY_SOURCE

Allan McRae allan at archlinux.org
Mon Jun 28 01:22:20 EDT 2010


On 28/06/10 14:56, Andres P wrote:
> On Mon, Jun 28, 2010 at 12:11 AM, Allan McRae<allan at archlinux.org>  wrote:
>>> +/* error handling in a single fn please */
>>
>> Seriously, this is one of the my useless pieces of function documentation I
>> have ever seen.
>>
>
> Call it wishful thinking?
>
>>> +void vw_asprintf(char **str, const char *fmt, ...)
>>
>> Why "vw_"?  We have several other similar wrappers called pm_<foo>  in util.c
>> already, so stick with that naming for consistency.
>>
>> Also, add the function proto to util.h.
>>
>
> I hope you don't do that since noone that includes util.h uses
> asprintf. This wrapper is useless outside util.c for the time being.
>
> Obviously prefixing it with pm_ is retarded since, as I said, it gets no use
> outside were it's defined...

So, if someone ever wants to use it outside util.c, we need to rename 
the function and add its header.  Call it forward thinking to do it 
properly the first time.   Any decent compiler removes unneeded 
functions on any include anyway so there is no loss in doing this but 
there is potential future gain.

I'd say not doing that way is "retarded", but really there is no call to 
start being offensive around here.

>> As an aside, it was obvious that this was something I was working on. So
>> duplicating the work is a waste of both of our time.   On that note, do not
>> bother resubmitting with my comments addressed as it would make the patch
>> almost identical to what I have locally.
>>
>
> Um, I explicitely said in the patch that I disagreed with it. If that's not
> indicative that I don't intend for this to be a commit, I don't know what is. I
> was showing you how to do it with a function without copy pasting everything.

No, you said:
I strongly disagree with making this void, but until all these void
funcions get a rework then I guess this is better than outright ignoring.

That tells me that this is a commit up for consideration as it improves 
the current situation.  Also, submitting a whole patch in proper git 
format indicates that you want it considered.

Or were you assuming that I did not understand you previous "make a
asprintf wrapper" comment...

Allan


More information about the pacman-dev mailing list