Re: [PATCH] F2FS support

[Andrei Borzenkov]

В Sat, 28 Mar 2015 13:43:18 -0700
Jaegeuk Kim <address@hidden> пишет:

> Hi Andrei,
> 
> On Sat, Mar 28, 2015 at 10:31:55AM +0300, Andrei Borzenkov wrote:
> > В Tue, 24 Mar 2015 01:19:00 -0700
> > Jaegeuk Kim <address@hidden> пишет:
> > 
> > >  * Makefile.util.def: Add f2fs.c.
> > >  * doc/grub.texi: Add f2fs description.
> > >  * grub-core/Makefile.core.def: Add f2fs module.
> > >  * grub-core/fs/f2fs.c: New file.
> > >  * tests/f2fs_test.in: New file.
> > >  * tests/util/grub-fs-tester.in: Add f2fs requirements.
> > > 
> > 
> > It's not the most useful commit message. Better would be short
> > explanation of use cases and intended platforms. I'm curious here -
> > F2FS is intended for raw flash access, on which platform(s) grub has
> > access to such devices? 
> 
> I just followed the commit convention in grub.git.

It has changed meanwhile. We are using normal git conventions now.

> > > +static grub_err_t
> > > +grub_f2fs_read_sb (struct grub_f2fs_data *data, int block)
> > > +{
> > > +  grub_disk_t disk = data->disk;
> > > +  grub_uint64_t offset;
> > > +  grub_err_t err;
> > > +
> > > +  if (block == 0)
> > > +    offset = F2FS_SUPER_OFFSET;
> > > +  else
> > > +    offset = F2FS_BLKSIZE + F2FS_SUPER_OFFSET;
> > > +
> > 
> > Please name it "secondary" or similar instead of "block" to avoid
> > confusion. You do not really want to read arbitrary block, right?
> >

Actually it makes more sense just to pass offset directly to eliminate
useless computation.