Re: Remaining violations of MISRA Rule 7.4

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

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)