[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [BusyBox] coreutils/date.c buffer [PATCH]
From:       Rob Landley <rob () landley ! net>
Date:       2005-05-28 22:52:06
Message-ID: 200505281852.06791.rob () landley ! net
[Download RAW message or body]

On Thursday 26 May 2005 05:18 pm, Shaun Jackman wrote:
> > Shouldn't the RELEASE_CONFIG_BUFFER macro have the
> > conditional on CONFIG_FEATURE_CLEAN_UP built-in?
>
> My thought is no; my reasoning is that the BUFFER might be allocated
> in an inner loop, in which case you'd want the buffer to be freed
> regardless of the CLEAN_UP setting so that the inner loop does not run
> out of memory.

Yeah, but this example isn't in an inner loop.

For allocations on the stack, the clean up is a NOP.  To me, that argues that 
we may just want to always call RELEASE_CONFIG_BUFFER unconditionally, 
because in the malloc/free case we already told it to use more memory, and 
the on-stack version is an empty macro.  (Even some loops that call it may be 
optimized away by the compiler, if written right.)  I've edited this patch to 
do that, since I had to do the whitespace cleanup again anyway.

This is actually a larger cleanup issue: we need to audit ALL our allocations 
and see where using CONFIG_BUFFER and FEATURE_CLEAN_UP make sense.  And 
that's not going to be this release.  In the mean time, I've applied the 
patch, _AND_ a note on the TODO list that we should deal with all this 
someday.

Applied.

Rob
_______________________________________________
busybox mailing list
busybox@mail.busybox.net
http://codepoet.org/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]