On 05/01/10 12:52, Joerg Schilling wrote:
> James Carlson <carls...@workingcode.com> wrote:
>> The above will need to be rewritten to conform with the style used in
>> the current OpenSolaris libc. That means:
>>
>> - none of this "schily" stuff
>>
>> - remove "register"
>>
>> - remove the unneeded ifndef
>>
>> - remove "EXPORT"
>>
>> - use ANSI C not K&R function declaration
>>
>> - prefer to use good variable names if possible; "s" isn't.
>
> You Most of the code I see from Sun does not follow
> these rules and the variable names are of course OK in a piece of code that
> is easy to read (all string sources in ON use tose 's' variables.....
> Note that K&K function _implemetations_ are needed for portability and that
> there is no benefit from using ANSI C here as there are ANCI C function
> _declarations_.
>
> I hope you know the difference between a function "declaration" and a function
> "implementation".
Read the C style guide on the web site and try again.
>> - prefer to use only needed headers; probably <sys/types.h>
>> and <string.h>.
>
> Well, in contrary to what's in Solaris, I write portable code. For a
> programmer, it should be easy to convert to other include files.
Irrelevant.
If you're trying to integrate code into OpenSolaris, you have to follow
the conventions in OpenSolaris. You can't just make up your own.
>> - code is incomplete; need updates to header files, Makefiles,
>> packaging, and library control files (mapfiles).
>
> See above.
Still missing, and a key part of the review. You're nowhere near ready
for integration into OpenSolaris if you haven't done at least this bit
of work.
> if (++len == 0) { /* unsigned overflow */
> len--;
> while (len-- > 0 && *rs++ != '\0')
> ;
> if (len == 0)
> return (rs - s);
> return (--rs - s);
Better.
>> It may be easier just to make "len == 0" a single special case on entry:
>>
>> if (len == 0)
>> return (0);
>> while (*rs++ != '\0' && --len > 0)
>> ;
>> if (len == 0)
>> return (rs - s);
>> else
>> return ((rs - s) - 1);
>
> Your code is incorrect an not acceptable to me.
>
> Your code tries to access more than "len" chars from the string, which is
> forbidden by the standard. Note that there is similar incorrect code in libc
> from Solaris that either accesses more data than permitted or does not work
> correctly with len == ~0.
No, it doesn't access more than "len" characters from the string under
any circumstances. You are wrong.
Let's suppose len is 0. That case is obvious -- immediate exit without
accessing anything. Proven.
Let's suppose len is 1 and the first byte is non-zero. We access one
character at the first location, then increment rs (which involves no
access to the string at all). We then pre-decrement len. It's now 0.
0>0 is false, so the loop terminates. No more accesses to the string
exist in the function, and we've accessed exactly one byte. Proven.
Let's suppose len is 1 and the first byte is zero. We access one
character at the first location, then increment rs. We do nothing else,
because 0 != '\0' is false. No more accesses exist in the code, and
we've accessed exactly one byte. Proven.
Let's suppose len is 2 and the first two bytes are non-zero. We access
one character at the first location, then increment rs. We then
pre-decrement len. It's now 1 and 1>1 is true, so we go again. We
access the second character at the second location, then increment rs.
We pre-decrement len. It's now 0, and we're done. Two bytes accessed.
Proven.
Continue for N. Proof by induction.
The code that I've written is correct, simpler, and smaller.
More generally, attacking your reviewers is amazingly bad style, and
will earn you nothing from me. I'm offended enough that I'll state
plainly: you may not use my name as your code reviewer on your
submission. Find someone else who will put up with this arrogant nonsense.
> You may like to check other strn*() functions in libc for this reason.
Many are legacy bits of code. If you can at least fit into that legacy
style, I'd be ok with it, but I suspect that the gatekeepers and RTI
advocates would not. New code is supposed to conform with the C style
guide.
However, inventing yet another alien style in OpenSolaris is just plain
out of scope. This isn't just someone's private plaything.
> BTW: Other missing code in libc:
>
> wcsncat()
> wcsncpy()
> wcsdup()
> wcsnlen()
>
> It have it....
Sounds great.
--
James Carlson 42.703N 71.076W <carls...@workingcode.com>
_______________________________________________
opensolaris-code mailing list
opensolaris-code@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/opensolaris-code
