[PATCH 1/9] dosfs: UTF-8 Support: UI, backwards compatibility

[Sebastian Huber]

On 06/03/2013 02:14 AM, Chris Johns wrote:
>> +    if (name_size>  0) {
>> +      /*
>> +       * Finally convert from UTF-8 to codepage
>> +       */
>> +      name_size_tmp = sizeof ( name_to_format_buf );
>> +      eno = (*converter->handler->utf8_to_codepage) (
>
> I have not been over all the code to check so I will ask. Is there always a set
> of converters even for the default case ? These pointers are never checked.

Yes, the function pointers are always valid.  Just like the file system 
operations and handlers.

>
>> +        converter,
>> +        name_ptr,
>> +        name_size,
>> +&name_to_format_buf[0],
>> +&name_size_tmp);
>> +      if ( eno != 0 ) {
>> +        /* The UTF-8 name my well be long name, for which we now want to
>    ... name may well be ..
>
>> +
>> +static int msdos_default_utf8_to_codepage(
>> +  rtems_dosfs_convert_control *super,
>> +  const uint8_t               *src,
>> +  const size_t                 src_size,
>> +  char                        *dst,
>> +  size_t                      *dst_size
>> +)
>> +{
>> +  int    eno = 0;
>> +  size_t bytes_to_copy = MIN( src_size, *dst_size );
>> +
>> +  (void) super;
>
> Why ? If this is for compiler warning please comment this is the case.

This is the standard way to get rid of unused parameter warnings without using 
GCC specific attributes.

>
>
> This is the default destroy function and it free's 'super'..
>
>> +
>> +static void msdos_default_destroy(
>> +  rtems_dosfs_convert_control *super
>> +)
>> +{
>> +  free( super );
>> +}
>> +
>
> ... and this code assumes 'super' is always first ...
>
>> +
>> +typedef struct {
>> +  rtems_dosfs_convert_control super;
>> +  uint8_t buffer[MSDOS_NAME_MAX_LFN_BYTES];
>> +} msdos_default_convert_control;
>> +
>> +rtems_dosfs_convert_control *rtems_dosfs_create_default_converter(void)
>> +{
>> +  msdos_default_convert_control *self = malloc( sizeof( *self ) );
>> +
>> +  if ( self != NULL ) {
>
> .. because this code references the element and sure it worked because it is
> first I just wonder if it could be more robust. At least a comment in the
> struct stating do not move the super field would be a good idea.

More robust would be to use C++.  This pattern is used everywhere in C, but we 
can add a comment.

-- 
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.