[PATCH] jffs, clang: avoid some warnings of used uninitialized ptr
Sebastian Huber
sebastian.huber at embedded-brains.de
Fri Oct 5 12:01:33 UTC 2018
On 05/10/2018 13:43, Daniel Hellstrom wrote:
> 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) {
> ^~~~~~~~
We have
int pointed = 0;
and the address of pointed is not taken, so this condition will be
always true. I am not sure what the constraints of the
-Wsometimes-uninitialized warnings are. Maybe they should trigger in
case code modifications elsewhere might lead to the offending condition.
Coverity Scan doesn't complain about this one.
Please have a look at this patch:
https://lists.rtems.org/pipermail/devel/2018-October/023190.html
In contrast to the buffer = NULL initialization this patch entirely
avoids the problematic path which must the compiler follow to deduces
this warning.
--
Sebastian Huber, embedded brains GmbH
Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail : sebastian.huber at embedded-brains.de
PGP : Public key available on request.
Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
More information about the devel
mailing list