[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] earlycpio: constify find_cpio_data()'s "data" parameter
On Mon, Oct 28, 2024 at 4:51 PM Jan Beulich <jbeulich@xxxxxxxx> wrote: > > On 28.10.2024 17:45, Andrew Cooper wrote: > > On 28/10/2024 4:25 pm, Jan Beulich wrote: > >> On 28.10.2024 17:18, Andrew Cooper wrote: > >>> On 28/10/2024 4:12 pm, Jan Beulich wrote: > >>>> On 28.10.2024 17:07, Andrew Cooper wrote: > >>>>> On 28/10/2024 4:03 pm, Jan Beulich wrote: > >>>>>> As with 9cbf61445cda ("xen/earlycpio: Drop nextoff parameter"): While > >>>>>> this is imported from Linux, the parameter not being pointer-to-const > >>>>>> is > >>>>>> dubious in the first place and we're not plausibly going to gain a > >>>>>> write > >>>>>> through it. > >>>>>> > >>>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> > >>>>> You haven't tried compiling this, have you? > >>>> Of course I have. Is there any subtlety with compiler versions? Or what > >>>> else am I missing? > >>> struct cpio_data's copy of this field is non-const (which you keep on > >>> noting that new compilers will object to), > >> New compilers? I'm afraid I'm missing context. With gcc14 the patch builds > >> fine. I didn't try _older_ ones (but I see no reason why they might object; > >> see below). > >> > >>> and you can't change that > >>> without breaking the build in microcode. > >> I don't need to change that, "thanks" to > >> > >> cd.data = (void *)dptr; > >> > >> casting away const-ness. That is - compilers ought to be fine with the > >> change; Misra won't like it. > > > > You have literally complained about patches of mine on the grounds of > > "GCC is about to start caring about casting away const on a void pointer". > > I still don't remember what context this was in, I'm sorry. > > > So which is it. > > I'm not adding any such casts; the (potentially problematic) cast is > there already. I therefore still don't see what's wrong with the patch. > You usually don't want some const data to be silently transformed to no-const data. In this case the "find_cpio_data" is getting a no-const pointer "data" and returning it into "cpio_data.data". As "cpio_data.data" is no-const for the previously stated rule the initial data (that is "data" pointer) should not be const. Internally you change from no-const to const with the assignment to "p" and than "dptr". However the "find_cpio_data" function has knowledge of the original no-const so it uses that knowledge for the no-const conversion done by "cd.data = (void *)dptr". That makes that conversion less "silent". > >>> Nothing of this form can be taken until the constness is consistent in > >>> microcode, after which yes it can mostly become const. > >> We can move there in steps, can't we? > > > > Or you can stop trying to insist that I rebase around an > > incorrect/incomplete patch, just for the sake of the const of one void > > pointer, which can still be laundered by this function. > > Okay, I won't insist; take my ack as unconditional one. I still consider > it a bad precedent though that we'd set, when elsewhere we ask for const- > correctness wherever possible. > > > Especially when you could wait the ~day it will take to get an > > otherwise-good series in, and then change cpio and get all of the const > > problems in one go. > > If that turns out to be true, all will indeed be fine in the end. Question > is whether we really want to diverge earlycpio.c by more than minimal > changes. > > Jan > Frediano
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |