change log for rtems (2010-06-15)

[Chris Johns]

On 16/06/10 7:23 PM, 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.  The result type of the expression is uint64_t
> (except for machines with sizeof(int)>  8).
>
>> To watch its brokenness, revert my change and compile RTEMS CVS HEAD
>> multilib'ed for the sparc64 and the avr.
>>

Sure. Could you please post the error ? I do not object to the need for 
a change just the implementation provided is also broken.

Do we need this change on 4.10 ?

>> You will notice  warnings related to printfs using this macro.
>
> Can you please post one of these warnings.
>
>> 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.
>
>>
>> That said, I am inclined to think these macros should be replaced with
>> (type-safe) functions .
>
> Yes.
>

This is one solution. I will review the code in question and see what is 
needed. If the code has performance issue it may pay to save the result 
in a uint64_t value. If it is not performance sensitive then a function 
is fine.

>>> The original and I think correct code created an intermediate value in
>>> compiler that is 64bits in size so the maths would not truncate. The
>>> 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.
>

Agreed. Wonder how a compact video recorder handles large files without 
a 64bit file system. Embedded systems and 64bit file systems are need 
because the storage media available these days is rather large and so 
are some of the files.

Chris