On Sat, Feb 2, 2013 at 5:05 AM, Marcus Meissner <mar...@jet.franken.de>wrote:
> On Fri, Feb 01, 2013 at 03:48:27PM -0800, Juan Lang wrote: > > On Fri, Feb 1, 2013 at 3:45 PM, Juan Lang <juan.l...@gmail.com> wrote: > > > > > Hi Marcus, > > > > > > - add_oid_to_usage(usage, ptr); > > > + usage = add_oid_to_usage(usage, ptr); > > > > > > This looks fine, but would you mind making the same change on line 337? > > > > > > Actually, perhaps I hit sent too early. If this memory allocation > fails, > > which is the only situation under which add_oid_to_usage doesn't just > > return its first parameter, it'll immediately crash on the next > invocation > > with a NULL pointer dereference. > > > > I'm not sure it's worth all the trouble in an out of memory situation. > > Perhaps just remove the return value and let the caller crash. > > Actually the loop around checks that as a condition and would lead to > return NULL: > > for (ptr = usageStr, comma = strchr(ptr, ','); usage && ptr && > *ptr; > > For the second one the loop around does not catch it. > > I think the add_oid_to_usage() should not even do it this way and not > touch "usage" > at all, but instead return a memory allocation error and let the caller > handle it:/ > > Or just let it crash. > Yeah, I think just letting it crash is better than trying to sort it all out. Thanks, and sorry for the confusing and buggy code :/ --Juan
