Re: [Xen-devel] [PATCH V4 09/15] Add arch specific module handling to read_file()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.09.14 at 02:51, <roy.franz@xxxxxxxxxx> wrote:
> @@ -398,18 +397,39 @@ static CHAR16 *__init point_tail(CHAR16 *fn)
>              break;
>          }
>  }

Missing blank line here.

> +/*
> + * Truncate string at first space, and return pointer
> + * to remainder of string.
> + */
> +static char * __init truncate_string(char *s)

"split" would seem a more correct naming to me than "truncate".

> +{
> +    while ( *s && !isspace(*s) )
> +        ++s;
> +    if ( *s )
> +    {
> +        *s = 0;
> +        return(s + 1);
> +    }
> +    return(NULL);

No parentheses here please.

>          if ( !EFI_ERROR(ret) && file->size != size )
>              ret = EFI_ABORTED;
> @@ -469,7 +487,7 @@ static bool_t __init read_file(EFI_FILE_HANDLE 
> dir_handle, CHAR16 *name,
>      {
>          PrintErr(what);
>          PrintErr(L" failed for ");
> -        PrintErrMesg(name, ret);
> +        PrintErrMesg(name.w, ret);
>      }
>  
>      return 1;
> @@ -525,7 +543,13 @@ static char *__init get_value(const struct file *cfg, 
> const char *section,
>              break;
>          default:
>              if ( match && strncmp(ptr, item, ilen) == 0 && ptr[ilen] == '=' )
> -                return ptr + ilen + 1;
> +            {
> +                ptr += ilen + 1;
> +                /* strip off any leading spaces */
> +                while ( *ptr && isspace(*ptr) )
> +                    ptr++;
> +                return ptr;
> +            }
>              break;
>          }
>          ptr += strlen(ptr);
> @@ -684,24 +698,26 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
> *SystemTable)
>          gop = NULL;
>  
>      /* Read and parse the config file. */
> -    if ( !cfg_file_name )
> +    if ( !cfg_file_name.w )
>      {
>          CHAR16 *tail;
>  
> -        while ( (tail = point_tail(file_name)) != NULL )
> +        while ( (tail = point_tail(file_name.w)) != NULL )
>          {
>              wstrcpy(tail, L".cfg");
> -            if ( read_file(dir_handle, file_name, &cfg) )
> +            if ( read_file(dir_handle, &cfg, w2s(&file_name)) )

No - this conversion is lossy. So far w2s() is being use _only_ when
there's no alternative (i.e. when the final consumer expects ASCII).

>                  break;
>              *tail = 0;
>          }
>          if ( !tail )
>              blexit(L"No configuration file found.");
>          PrintStr(L"Using configuration file '");
> -        PrintStr(file_name);
> +        s2w(&file_name);
> +        PrintStr(file_name.w);
>          PrintStr(L"'\r\n");
> +        efi_bs->FreePool(file_name.w);

And eliminating the conversion above would make the ugliness here
unnecessary too.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel