[PATCH 2/2] mkfs.ubifs: use libubi to format UBI volume

Corentin Chary corentin.chary at gmail.com
Mon May 25 04:23:03 EDT 2009


On Mon, May 25, 2009 at 9:58 AM, Artem Bityutskiy
<dedekind at infradead.org> wrote:
>
> I know this is nit-picking, but would be nicer to close libubi
> in case of errors. Namely, in the second and third checks.

In case of error, close_target() will close libubi. (if(ubi) libubi_close(ubi)).
But maybe you want to explicitly close libubi here ?


>> +static int write_empty_leb(int lnum, int dtype)
>>  {
>> -     return write_leb(lnum, 0, leb_buf);
>> +  return write_leb(lnum, 0, leb_buf, dtype);
>>  }
>
> You screwed the indentation here a bit.

Arg... M-x c-set-style gnu :/

>> -     err = stat(root, &root_st);
>> -     if (err)
>> -             return sys_err_msg("bad root file-system directory '%s'", root);
>> +     if (root) {
>> +             err = stat(root, &root_st);
>> +             if (err)
>> +                     return sys_err_msg("bad root file-system directory '%s'"
>> +                                        , root);
>
> This is a little ugly. Could you please leave the comma at close to the
> " symbol, and move "root" to the second line. This is just how the rest
> of the code is written. Let's be consistent.

The comma is just at the 80 column limit.
Using Lindent, we get:
                        return
                            sys_err_msg("bad root file-system directory '%s'",
                                        root);

Is this ok ?

>
>> --- a/mkfs.ubifs/ubifs.h
>> +++ b/mkfs.ubifs/ubifs.h
>> @@ -366,6 +366,9 @@ struct ubifs_info
>>       int dead_wm;
>>       int dark_wm;
>>
>> +     struct ubi_dev_info di;
>> +     struct ubi_vol_info vi;
>
> For consistency, you should probably comment these new fields.
Ok



-- 
Corentin Chary
http://xf.iksaif.net - http://uffs.org



More information about the linux-mtd mailing list