On 28/06/10 14:56, Andres P wrote:
On Mon, Jun 28, 2010 at 12:11 AM, Allan McRae<[email protected]>  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

Reply via email to