memcmp.3: clarify return value
ClosedPublic
Actions

Authored by emaste on Wed, Nov 20, 3:46 PM.

Details

Reviewers

getz
fuz

Commits

rG5cc53d79655b: memcmp.3: Clarify return value

Summary

The return value is not required to be the difference between the
differing bytes, only less than zero, zero, or greater than zero.

Event:          Kitchener-Waterloo Hackathon 202406

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

emaste requested review of this revision.Wed, Nov 20, 3:46 PM

emaste created this revision.

We have always guaranteed the extended behaviour. I don't believe we should change it nor stop documenting it.
Note that as to my knowledge, arm64 currently does not conform to the extended behaviour. This went undiscovered for a long time due to the absence of any unit test checking the return value. There is a unit test now.

Maybe instead we should document in the STANDARDS section that ISO 9899 and POSIX guarantee just the positive/zero/negative behaviour.

fuz added a subscriber: getz.Wed, Nov 20, 3:50 PM

I don't believe we should change it nor stop documenting it.

I disagree: this is a standard C function. If our implementation goes beyond the requirements and FreeBSD developers make use of this it just means that we produce non-portable code. I imagine the reason we did not discover arm64's divergence is primarily because memcmp's return value is really only tested as == 0 or != 0 or < 0 or > 0.

lib/libc/string/memcmp.3
58–66

andrew added a subscriber: andrew.Wed, Nov 20, 4:06 PM

andrew added inline comments.

lib/libc/string/memcmp.3
48

missing word, update test

emaste marked an inline comment as done.Wed, Nov 20, 4:07 PM

Hard disagree on no longer testing the behaviour; removing it is an API break and we should test the API we ship.

If you don't want to document our extended behaviour, that is debatable. In descending order of preference:

document the behaviour fully, but note that this goes beyond what the standard says
don't mention the extended behaviour in the main body of the man page, but add a note that we guarantee more
don't mention it at all (really not a fan of that)

lib/libc/tests/string/memcmp_test.c
48 ↗	(On Diff #146749)	Removing that guarantee from code and unit test is an API break. We should not break the API like that. Removing the documentation is one thing, but breaking the API goes to far in any case. Guaranteeing the extended behaviour is cheap or free in all case I know of and our unit tests should validate that.