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

[Chris Johns]

Ralf Kirchner wrote:
>
> +ssize_t
> +msdos_filename_utf8_to_long_name_for_save (
  ...
> +        if (name_size_tmp<= (MSDOS_NAME_MAX_LNF_LEN * MSDOS_NAME_LFN_BYTES_PER_CHAR))

Can MSDOS_NAME_MAX_LNF_LEN * MSDOS_NAME_LFN_BYTES_PER_CHAR be shortened ?

There are a few places where this would help the line length. For example ..

> +ssize_t
> +msdos_filename_utf8_to_short_name_for_compare (
  ...
> +  uint8_t        name_normalized_buf[(MSDOS_SHORT_NAME_LEN +1) * MSDOS_NAME_MAX_UTF8_BYTES_PER_CHAR];

> +ssize_t
> +msdos_filename_utf8_to_short_name_for_save (
  ...
> +    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.

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

> +    rtems_dosfs_convert_control *super =&self->super;
> +
> +    super->handler =&msdos_default_convert_handler;
> +    super->buffer.data =&self->buffer;
> +    super->buffer.size = sizeof( self->buffer );
> +  }
> +
> +  return&self->super;
> +}

Chris