> On Jan 27, 2021, at 4:32 PM, Jan Beulich <[email protected]> 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. -George
