This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Implement C11 annex K?


Rich Felker <dalias@libc.org> writes:

> Indeed, I misread. Since you're already computing the lengths to
> allocate the right storage, but don't have storage to keep the computed
> lengths, it's inefficient and ugly to compute them again.  This is one
> case where GNY stpcpy, or using snprintf as a substitute for it, is
> probably the right solution.

> Certainly strlcpy/strlcat are not needed here since the length is known
> to be correct.

The second part is what I've often suspected is behind the resistence to
strlcpy and strlcat.  Folks making that argument are confident enough in
their ability to write C code and length calculation that they don't feel
the need for any further safety measure.

Based on your contributions here, you're clearly a more expert C
programmer than I am, and I believe this probably works well for you.  I
do not have the same level of confidence in my own ability.  :)  I would
much prefer to add the additional check and take a small performance hit
for the sake of a bit more robustness in the face of my own mistakes.

Here's the loop with strlcpy/strlcat with assert checking, for comparing
apples to apples:

    strlcpy(string, vector->strings[0], size);
    for (i = 1; i < vector->count; i++) {
        nbytes = strlcat(string, seperator, size);
        assert(nbytes < size);
        nbytes = strlcat(string, vector->strings[i], size);
        assert(nbytes < size);
    }

(This can obviously be simplified if you have a version of assert that
always runs the code inside the assert.)

The snprintf code that I came up with is about as bad as the memcpy code:

    offset = snprintf(string, size, "%s", vector->strings[0]);
    for (i = 1; i < vector->count; i++) {
        nbytes = snprintf(string + offset, size - offset, "%s", separator);
        assert(nbytes < size - offset);
        offset += nbytes;
        nbytes = snprintf(string + offset, size - offset, "%s",
                          vector->strings[1]);
        assert(nbytes < size - offset);
        offset += nbytes;
    }

The stpcpy code I came up with is:

    end = stpcpy(string, vector->strings[0]);
    for (i = 1; i < vector->count; i++) {
        end = stpcpy(end, separator);
        end = stpcpy(end, vector->strings[1]);
    }

This is at least comparable in readability to the assert-less
strlcpy/strlcat original, and more readable than the version with asserts,
but has no additional safety check.

So... if you're an expert C programmer who is confident in your ability to
write the size calculation and always get it right, stpcpy is at least
arguably the most readable.  But for the rest of us, I'm still quite
unconvinced by the arguments against strlcpy/strlcat.

I should stress that this is real code.  I've been following this list for
years, and have a lot of respect for the people who post here.  So, when
people said repeatedly that strlcpy and strlcat are unnecessary and
shouldn't be used, I went to some effort to find other ways of writing all
the code that I had that used them.  And I was largely successful;
asprintf in particular is a better solution for most places I was using
them.  But I was left with a small set of cases of dynamic string
creation, like this one, and I could not convince myself that any of the
approaches other than strlcpy/strlcat were better.  And that's still where
I am.

I would really like glibc to include strlcpy and strlcat so that I can
stop including my own versions for use in this specific situation.  Or,
alternately, for someone to explain to me why I'm wrong in the form of
more readable and maintainable code that doesn't use strlcpy/strlcat.

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]