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