Re: Remaining violations of MISRA Rule 7.4

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

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.

~Andrew