[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Fix error: array subscript has type 'char' [and 1 more messages]

On 27.01.2021 17:52, George Dunlap wrote:
>> On Jan 27, 2021, at 4:32 PM, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>> On 27.01.2021 17:21, Ian Jackson wrote:
>>> Jan Beulich writes ("Re: [PATCH] Fix error: array subscript has type 'char' 
>>> [and 1 more messages]"):
>>>> I don't think I've ever come across that part of a platform
>>>> API/ABI spec. Instead my understanding so far was that good
>>>> platform headers would be ignorant of the user's choice of
>>>> char's signed-ness (wherever compilers offer such a choice,
>>>> but I think all that I've ever worked with did). At the very
>>>> least gcc's doc doesn't warn about any possible
>>>> incompatibilities resulting from use of -fsigned-char or
>>>> -funsigned-char (or their bitfield equivalents, for that
>>>> matter).
>>> Well, I've considered this and I still don't think changing to
>>> -funsigned-char is good idea.
>>> Are you OK with me checking in the current patch or should I ask the
>>> other committers for a second opinion ?
>> For the changes to tools/ it's really up to you. For the change
>> to xen/tools/symbols.c I could live with it (for being user
>> space code), but I still think adding casts in such a place is
>> not necessarily setting a good precedent. So for this one I'd
>> indeed appreciate getting another opinion.
> My thoughts:
> * On the whole, the risk of an incompatibility with system headers does seem 
> higher than the risk of casting a value which is known not to be EOF
> * Such a change doesn’t seem like the kind of thing we should ask Manuel to 
> do, when a simpler change will do the trick
> * At any rate it doesn’t seem like a good thing to experiment with in the 
> week before the feature freeze.

Well, okay then, I withdraw my implied nak under these conditions.




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.