Re: Implement C11 annex K?

[Russ Allbery <eagle at eyrie dot org>]

Rich Felker <dalias@libc.org> writes:
> On Fri, Aug 15, 2014 at 07:25:51PM -0700, Russ Allbery wrote:

>> What generally you get from strlcpy/strlcat is turning buffer overflows
>> into truncations (which one can then check for, but which a lot of code
>> does not).  I generally agree with you that this is useful for legacy
>> code that uses a lot of static buffers.  I also generally agree with
>> Paul that static buffers are *themselves* a design flaw that should be
>> avoided, and it's not entirely clear to me that it's a good idea to
>> modify code to add strlcpy/strlcat and stop there, rather than
>> modifying it to stop using static buffers entirely.

> If they're really using _static_ storage, that's a design bug, yes.  But
> in practice they're often using automatic-storage buffers with a
> constant size limit. This is rarely a design bug unless the requirements
> actually require supporting arbitrary-length data; continually
> allocating memory until you OOM is, in practice, a much bigger design
> bug.

Well, I don't think you should be assuming that any code that uses dynamic
buffers continues to allocate memory until gets OOM.  That's certainly not
a necessary property of code that uses dynamic memory allocation.  But I
think we're largely in agreement here.

>> And, honestly, if I were working on such legacy code, I would be very
>> tempted to use asprintf into a temporary variable to do whatever I
>> want, check the total length when finished to see if it will fit, and
>> then either return an error or memcpy the results into the static
>> buffer.  That doesn't change the calling contract, but gives you many
>> of the safety benefits of dynamic allocation.

> It does change the calling contract. It changes a function which cannot
> fail to a function which has a failure case that it needs to be able to
> report to the caller, which is a radically different contract because
> now the caller has to check whether it succeeded or failed.

Er, sorry, you seem to have lept to a conclusion that I hadn't intended,
probably because I was insufficiently clear.  I was assuming a legacy
contract such as the calling contract for getcwd, where one returns an
error if the passed buffer is too small.  (Yes, I know there are better
functions available now that does the same thing; just take it as an
example of the sort of contract that's often seen in legacy code.)  Even
if you're stuck with that contract and can't do the trick that was applied
to getcwd to do dynamic allocation, I would still be tempted to work in
terms of dynamic allocation internal to the fuction and only check to see
if the value will fit at the end of the operation.  It's usually safer and
cleaner.

> And if you make the function abort the whole program on error (a common
> disgusting solution to this problem which a depressing number of people
> do) you've still changed the interface contract: now a function which
> previously always returned (successfully, no less) might return or might
> abort the program.

Yes, we're in firm agreement here.  I only use abort() and assert() (at
least in library code) for cases where the triggering of those functions
represents a serious coding error *inside that library*, such as
double-checking a length calculation or some similar invariant to provide
a second level of protection against coding errors.  Not for reporting
errors from the caller.

-- 
Russ Allbery (eagle@eyrie.org)              <http://www.eyrie.org/~eagle/>