Re: [PATCH] xen/lib: Fix strcmp() and strncmp()

[Ian Jackson <iwj@xxxxxxxxxxxxxx>]

Andrew Cooper writes ("Re: [PATCH] xen/lib: Fix strcmp() and strncmp()"):
> On 27/07/2021 19:47, Jane Malalane wrote:
> > -   register signed char __res;
> > +   unsigned char *csu = (unsigned char *)cs;
> > +   unsigned char *ctu = (unsigned char *)ct;
> 
> So there was actually one final thing, but it is holiday season, hence
> the lack of replies from others.

Oh.

> We should not be casting away const-ness on the pointers, because that
> is undefined behaviour and compilers are starting to warn about it. 

I don't think casting away const is UB.  Perhaps you (and perhaps
others) are seeing this in 6.3.2.3(2):

 | For any qualifier q, a pointer to a non-q-qualified type may be
 | converted to a pointer to the q-qualified version of the type; the
 | values stored in the original and converted pointers shall compare
 | equal.p

This does indeed define the meaning of *adding* qualifiers to a
pointer type but not define the meaning of removing them.  But that
whole paragraph is almost redundant, because in 6.3.2.3(7):

 | A pointer to an object or incomplete type may be converted to a
 | pointer to a different object or incomplete type. If the resulting
 | pointer is not correctly aligned57) for the pointed-to type, the
 | behavior is undefined. Otherwise, when converted back again, the
 | result shall compare equal to the original pointer.

This defines the meaning of conversions of pointers to object types
(like char*) regardless of the qualifiers.

I read that as "a pointer to an object type or to an incomplete type".
But the precise reading doesn't matter because these pointers are
actually to objects.

There's also this in 6.7.3(5):

 | If an attempt is made to modify an object defined with a
 | const-qualified type through use of an lvalue with
 | non-const-qualified type, the behavior is undefined.  made to refer
 | to an object defined with a volatile-qualified type through

But there is no attempt to modify.  (Also this paragraph doesn't apply
because characters in string literals have type char, not type const
char, but 6.4.6(6) directly prohibits modification of characters in
string literals.)

6.2.7(2) says

 | All declarations that refer to the same object or function shall
 | have compatible type; otherwise, the behavior is undefined.

but I don't think these pointers variables are declarations of the
chars pointed to.

> Therefore, we want something like:
> 
> const unsigned char *csu = (const unsigned char *)cs;

Having said all thst, I agree that that not casting away const would
be better (especially if it generates compiler warnings).

I pushed it already.  If thios is UB we should revert it but as I say
I think it isn't, so we can wait for a followup.

Thanks,
Ian.