Re: Remaining violations of MISRA Rule 7.4

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2023-11-08 19:45, Andrew Cooper wrote:
> On 08/11/2023 4:24 pm, Nicola Vetrini wrote:
> > Hi everyone,
> >
> > I was looking at leftover violations for MISRA Rule 7.4:
> > 'A string literal shall not be assigned to an object unless the
> > object's type
> > is "pointer to const-qualified char" '
> >
> > You can see the referenced violations at [1] and [2].
> >
> > I think the ones in x86/setup.c can be taken care of either by making
> > an early return
> > from cmdline_cook, given that one caller never supplies a NULL
> > cmdline, while the other
> > properly takes care of the possibility of returning NULL, afaict.
> >
> >  static char * __init cmdline_cook(char *p, const char *loader_name)
> >  {
> > -    p = p ? : "";
> > +    if ( p == NULL )
> > +        return NULL;
> >
> > or changing the type of "loader" to const char*
> >
> >  void __init noreturn __start_xen(unsigned long mbi_p)
> >  {
> > -    const char *memmap_type = NULL;
> > -    char *cmdline, *kextra, *loader;
> > +    const char *memmap_type = NULL, *loader = NULL;
> > +    char *cmdline, *kextra;;
> >
> > as, as far as I can tell, it's never changed after
> >
> >     loader = (mbi->flags & MBI_LOADERNAME)
> >         ? (char *)__va(mbi->boot_loader_name) : "unknown";
> >
> > However, the one in xen/arch/arm/efi/efi-boot.h
> >
> > name.s = "xen";
> >
> > does not look to have a clear resolution
> > path, therefore I propose to deviate this with a SAF textual
> > deviation, whose justification
> > relies on the fact that the string is never modified afterwards.
> >
> > For the one in arm-uart.c from the discussion, I'm testing possible
> > solution with no code
> > changes, but if that doesn't work out, then I'm inclined towards a
> > deviation, as options
> > is never modified afterwards.
> >
> > What do you think?
> I've just rebased and pushed the residual from the past work (although 
> I
> missed the ARM EFI fix.)
>
> https://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen.git;a=commitdiff;h=0f06bab762f5201f3e00aaaee704c3c01f516b51
> https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1065699873
>
> I'm going to make a firm request that we fix this by activating
> -Wwrite-strings Xen wide, because that's by far and away the best way 
> to
> stop regressions creeping back in.
Yes, it seems the best solution. I forgot about that patch, sorry.

  drivers/char/arm-uart.c: In function 'dt_uart_init':
  drivers/char/arm-uart.c:81:17: error: assignment discards 'const' 
qualifier from pointer target type [-Werror=discarded-qualifiers]
     81 |         options = "";
        |                 ^

Here I was thinking about

     options = strchr(opt_dtuart, ':');
     if ( options != NULL )
         *(options++) = '\0';
-    else
-        options = "";

-    printk("Looking for dtuart at \"%s\", options \"%s\"\n", devpath, 
options);
+    printk("Looking for dtuart at \"%s\", options \"%s\"\n", devpath,
+           options ? options : "");

> In start_xen(), basically whatever goes.  All that's doing is 
> processing
> of one command line into another, and your version looks a bit neater
> than mine.
>
> The name.s cases (it's duplicated in x86 and ARM) are more tricky.  The
> compiler warning can be silenced by swapping name.s for name.cs but I
> have no idea whether Eclair can see through that piece of blatent 
> lying.
No it doesn't, because the type of the lhs is checked, and if it's 
constant then it's enough for the Rule.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)