[PATCH] jffs, clang: avoid some warnings of used uninitialized ptr

[Daniel Hellstrom]

On 2018-10-05 13:01, Sebastian Huber wrote:
> On 05/10/2018 12:44, Daniel Hellstrom wrote:
>>
>> On 2018-10-05 09:41, Sebastian Huber wrote:
>>> On 05/10/2018 09:38, Daniel Hellstrom wrote:
>>>>
>>>> On 2018-10-05 09:34, Sebastian Huber wrote:
>>>>> On 05/10/2018 09:25, Daniel Hellstrom wrote:
>>>>>> On 2018-10-05 09:12, Sebastian Huber wrote:
>>>>>>> On 05/10/2018 08:57, Daniel Hellstrom wrote:
>>>>>>>> This fixes the following test failures on LEON3 UP/SMP when
>>>>>>>> built using clang compiler:
>>>>>>>>   * fsjffs2gc01
>>>>>>>>   * jffs2_fserror
>>>>>>>>   * jffs2_fspermission
>>>>>>>>   * jffs2_fsrdwr
>>>>>>>>   * jffs2_fstime
>>>>>>>
>>>>>>> The problem is probably in the 
>>>>>>> rbtree_postorder_for_each_entry_safe(). Coverity Scan has also 
>>>>>>> an issue with this macro. It is on my todo list. Please do not 
>>>>>>> check in this patch.
>>>>>> Ok I will not push it. Yes I was also thinking so. To use 
>>>>>> offsetof() macro could be an option maybe, however the node type 
>>>>>> input is not known to the rbtree_postorder_for_each_entry_safe() 
>>>>>> function even though it only used from jffs2 code. 
>>>>>
>>>>> The Linux code uses typeof. We could do this also:
>>>>>
>>>>> diff --git a/cpukit/include/linux/rbtree.h 
>>>>> b/cpukit/include/linux/rbtree.h
>>>>> index 53c777e8c1..8fc575240f 100644
>>>>> --- a/cpukit/include/linux/rbtree.h
>>>>> +++ b/cpukit/include/linux/rbtree.h
>>>>> @@ -126,12 +126,12 @@ static inline struct rb_node *rb_parent( 
>>>>> struct rb_node *node )
>>>>>    for ( \
>>>>>      node = _RBTree_Postorder_first( \
>>>>>        (RBTree_Control *) root, \
>>>>> -      (size_t) ( (char *) &node->field - (char *) node ) \
>>>>> +      offsetof( __typeof__( *node ), field ) \
>>>>>      ); \
>>>>>      node != NULL && ( \
>>>>>        next = _RBTree_Postorder_next( \
>>>>>          &node->field, \
>>>>> -        (size_t) ( (char *) &node->field - (char *) node ) \
>>>>> +        offsetof( __typeof__( *node ), field ) \
>>>>>        ), \
>>>>>        node != NULL \
>>>>>      ); \
>>>>>
>>>>> It shouldn't be a big problem since at least GCC and clang 
>>>>> supports it. It is also a standard C++ operator. It is already 
>>>>> used in
>>>>>
>>>>> cpukit/libfs/src/jffs2/include/linux/list.h
>>>>
>>>> Ok, I will give it a try.
>>>>
>>>
>>> Thanks, please try this patch:
>>>
>>> https://lists.rtems.org/pipermail/devel/2018-October/023181.html
>>
>> Yes I can confirm it fixed all the 5 tests on gr712rc with clang. It 
>> also removed the two warnings in jffs2 code related.
>>
>> I have updated my patch to only include the third warning fix then:
>>
>> ---
>>  cpukit/libfs/src/jffs2/src/readinode.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/cpukit/libfs/src/jffs2/src/readinode.c 
>> b/cpukit/libfs/src/jffs2/src/readinode.c
>> index 519b0d6..ecdf335 100644
>> --- a/cpukit/libfs/src/jffs2/src/readinode.c
>> +++ b/cpukit/libfs/src/jffs2/src/readinode.c
>> @@ -35,7 +35,7 @@ static int check_node_data(struct jffs2_sb_info *c, 
>> struct jffs2_tmp_dnode_info
>>      struct jffs2_raw_node_ref *ref = tn->fn->raw;
>>      int err = 0, pointed = 0;
>>      struct jffs2_eraseblock *jeb;
>> -    unsigned char *buffer;
>> +    unsigned char *buffer = NULL;
>>      uint32_t crc, ofs, len;
>>      size_t retlen; 
>
> What is this for a warning? Some sort of uninitialized variable 
> warning? This code is identical to the Linux upstream. This is 
> probably a clang bug.
>
buffer will never be uninitialized from what I see, so yes maybe its a 
clang bug. This is the warning:


sparc-gaisler-rtems5-clang --pipe -DHAVE_CONFIG_H   -I.. 
-I/home/daniel/git/rtems/rcc-1.3/rtems/build/gr712rc_drvmgr_test_up/sparc-gaisler-rtems5/c/gr712rc/include 
-I/home/daniel/git/rtems/rcc-1.3/rtems/kernel/cpukit/include 
-I/home/daniel/git/rtems/rcc-1.3/rtems/kernel/cpukit/score/cpu/sparc/include 
-I/home/daniel/git/rtems/rcc-1.3/rtems/kernel/cpukit/libnetworking 
-I/home/daniel/git/rtems/rcc-1.3/rtems/kernel/c/src/../../cpukit/libfs/src/jffs2/include 
-Wno-pointer-sign -mcpu=gr712rc -O2 -g -ffunction-sections 
-fdata-sections -Wall -Wmissing-prototypes 
-Wimplicit-function-declaration -Wstrict-prototypes -Wnested-externs -MT 
src/jffs2/src/libjffs2_a-readinode.o -MD -MP -MF 
src/jffs2/src/.deps/libjffs2_a-readinode.Tpo -c -o 
src/jffs2/src/libjffs2_a-readinode.o `test -f 
'src/jffs2/src/readinode.c' || echo 
'/home/daniel/git/rtems/rcc-1.3/rtems/kernel/c/src/../../cpukit/libfs/'`src/jffs2/src/readinode.c
/home/daniel/git/rtems/rcc-1.3/rtems/kernel/c/src/../../cpukit/libfs/src/jffs2/src/readinode.c:80:6: 
warning: variable 'buffer' is used uninitialized whenever 'if' condition 
is false [-Wsometimes-uninitialized]
         if (!pointed) {
             ^~~~~~~~
/home/daniel/git/rtems/rcc-1.3/rtems/kernel/c/src/../../cpukit/libfs/src/jffs2/src/readinode.c:101:31: 
note: uninitialized use occurs here
         crc = crc32(tn->partial_crc, buffer, len);
                                      ^~~~~~
/home/daniel/git/rtems/rcc-1.3/rtems/kernel/c/src/../../cpukit/libfs/src/jffs2/include/linux/crc32.h:8:71: 
note: expanded from macro 'crc32'
#define crc32(val, s, len) cyg_crc32_accumulate(val, (unsigned char *)s, 
len)
                                                                       ^
/home/daniel/git/rtems/rcc-1.3/rtems/kernel/c/src/../../cpukit/libfs/src/jffs2/src/readinode.c:80:2: 
note: remove the 'if' if its condition is always true
         if (!pointed) {
         ^~~~~~~~~~~~~~
/home/daniel/git/rtems/rcc-1.3/rtems/kernel/c/src/../../cpukit/libfs/src/jffs2/src/readinode.c:38:23: 
note: initialize the variable 'buffer' to silence this warning
         unsigned char *buffer;
                              ^
                               = NULL
1 warning generated.