change log for rtems (2010-06-15)

[Ralf Corsepius]

On 06/16/2010 11:23 AM, Sebastian Huber wrote:
> On 06/16/2010 10:54 AM, Ralf Corsepius wrote:
>    
>> On 06/16/2010 10:26 AM, Chris Johns wrote:
>>      
>>> On 16/06/10 5:26 PM, Sebastian Huber wrote:
>>>        
>>>> On 06/15/2010 06:11 PM, rtems-vc at rtems.org wrote:
>>>> [...]
>>>>          
>>>>> diff -u rtems/cpukit/ChangeLog:1.2346.2.16
>>>>> rtems/cpukit/ChangeLog:1.2346.2.17
>>>>> --- rtems/cpukit/ChangeLog:1.2346.2.16    Tue Jun 15 08:35:48 2010
>>>>> +++ rtems/cpukit/ChangeLog    Tue Jun 15 10:17:42 2010
>>>>> @@ -1,5 +1,6 @@
>>>>>    2010-06-15    Ralf Corsépius<ralf.corsepius at rtems.org>
>>>>>
>>>>> +    * libfs/src/rfs/rtems-rfs-file-system.h: Remove bogus typecast.
>>>>>        * libfs/src/rfs/rtems-rfs-block.c,
>>>>> libfs/src/rfs/rtems-rfs-buffer.c,
>>>>>        libnetworking/nfs/bootp_subr.c: Misc. 64bit-compatibility fixes.
>>>>>        * posix/include/rtems/posix/pthread.h: Remove stray comment.
>>>>>
>>>>> diff -u rtems/cpukit/libfs/src/rfs/rtems-rfs-file-system.h:1.3
>>>>> rtems/cpukit/libfs/src/rfs/rtems-rfs-file-system.h:1.4
>>>>> --- rtems/cpukit/libfs/src/rfs/rtems-rfs-file-system.h:1.3    Thu
>>>>> Feb 25 23:54:59 2010
>>>>> +++ rtems/cpukit/libfs/src/rfs/rtems-rfs-file-system.h    Tue Jun 15
>>>>> 10:17:35 2010
>>>>> @@ -308,7 +308,7 @@
>>>>>     *
>>>>>     * @param _fs Pointer to the file system.
>>>>>     */
>>>>> -#define rtems_rfs_fs_size(_fs) (((uint64_t) rtems_rfs_fs_blocks
>>>>> (_fs)) * \
>>>>> +#define rtems_rfs_fs_size(_fs) (rtems_rfs_fs_blocks (_fs) * \
>>>>>                                    rtems_rfs_fs_block_size (_fs))
>>>>>
>>>>>    /**
>>>>>
>>>>>            
>>>> [...]
>>>>
>>>> The blocks and block_size fields are of type size_t, thus I don't
>>>> think that
>>>> this type cast is bogus.
>>>>
>>>>          
>>> Hmm indeed the change would seem broken.
>>>        
>> Well, this macro is broken. My change was broken, also.
>>      
> This macro is not broken.
It is - C.f. below.
>    The result type of the expression is uint64_t
> (except for machines with sizeof(int)>  8).
>    

Exactly this assumption is wrong.

The result of macros depends on the context they are being used in.

>> To watch its brokenness, revert my change and compile RTEMS CVS HEAD
>> multilib'ed for the sparc64 and the avr.
>>
>> You will notice  warnings related to printfs using this macro.
>>      
> Can you please post one of these warnings.
>    
I wonder why you are not able to reproduce them.

Here is one of them:

../../../../cpukit/libfs/src/rfs/rtems-rfs-format.c:587:13: warning: 
format '%llu' expects type 'long long unsigned int', but argument 2 has 
type 'size_t'
../../../../cpukit/libfs/src/rfs/rtems-rfs-format.c:587:13: warning: 
format '%llu' expects type 'long long unsigned int', but argument 2 has 
type 'size_t'


with (rtems-rfs-format.c)

586:    printf ("rtems-rfs: format: size = %llu\n",
587            rtems_rfs_fs_size (&fs));

and (rtems-rfs-file-system.h)

311: #define rtems_rfs_fs_size(_fs) (((uint64_t) rtems_rfs_fs_blocks 
(_fs)) * \
312:                                rtems_rfs_fs_block_size (_fs))

=> rtems_rfs_fs_size is broken. The uint64_t cast is dysfunctional.

>    
>> AFAIU, what happens is the compiler applying implicit type expansions
>> depending on the context this macro is being used. I.e. I am watching a
>> side-effect of macros being "type-unsafe".
>>      
> The compiler would also apply implicit type expansions in case of a function
> instead of the macro.
>    
No. A macro is caller context sensitive, a function is not.

 >>> The original and I think correct code created an intermediate value in
 >>> compiler that is 64bits in size so the maths would not truncate.
Cf. above.


>>> cast is needed and valid. The RFS is a 64bit file system.
>>>        
>> Yes, a serious regression, which needs to be addressed in future.
>>
>> 64bit filessystems on "pure 16bit or 32bit" targets and on systems which
>> don't need 64bit filesystems (e.g. an embedded controller with small
>> SDRAMS) don't make any sense.
>>
>> Ralf
>>
>>      
> 64-bit file systems makes sense on any machine
Rubbish.

64bit filesystems are
* non-applicable on machines without 64bit support.
* ineffective on many 16/32bit cpus.
* ineffective in applications when filesystem sizes are guaranteed to be 
small.
* a waste of resources on small systems.