On Tue, 16 Nov 2010, Eric Sandeen wrote:
-snip-
> >>> +
> >>> + struct struct_ext2_filsys *fs;
> >>> + struct unix_private_data *u_priv;
> >>> + errcode_t retval = 0;
> >>> + __uint64_t range[2];
> >>> + int blocksize;
> >>> +
> >>> + EXT2_CHECK_MAGIC(channel, EXT2_ET_MAGIC_IO_CHANNEL);
> >>> + u_priv = (struct unix_private_data *) channel->private_data;
> >>> + EXT2_CHECK_MAGIC(u_priv, EXT2_ET_MAGIC_UNIX_IO_CHANNEL);
> >>> +
> >>> + fs = (struct struct_ext2_filsys *) data;
> >>> + if (!fs)
> >>> + return EXT2_ET_INVALID_ARGUMENT;
> >>> +
> >>> + blocksize = EXT2_BLOCK_SIZE(fs->super);
> >>
> >> This seems a little convoluted; you pass in *data, which gets you the fs,
> >> from which you get the super, from which you get the blocksize,
> >> which is all that you ever actually use here:
> >>
> >>> + range[0] = (__uint64_t)(block);
> >>> + range[1] = (__uint64_t)(count);
> >>> + range[0] *= (__uint64_t)(blocksize);
> >>> + range[1] *= (__uint64_t)(blocksize);
> >>
> >> any reason to not just pass in the blocksize?
> >>
> >> Maybe this is for flexibility for not-linux, but I guess we don't know what
> >> they need anyway...?
> >>
> >> And if you do that you can change "u_priv" to "data" just to match the
> >> other handlers, perhaps.
> >
> > You're right, I was so closely following the example that I forget that
> > it can be done more simply. Does this looks good to you ?
> >
> > static errcode_t unix_discard(io_channel channel, unsigned long long block,
> > unsigned long long count, const void *blocksize)
> > {
> > unsigned int *bsize = (unsigned int *) blocksize;
>
> I think you can just pass in "int blocksize" right?
>
> and
>
> ret = manager->discard(fs->io, start, count, fs->blocksize);
>
> -Eric
>
You are probably right, there is no need for io_manager to pass other
information than just block, count and blocksize, so I'll change discard
definition for that.
Thanks!
-Lukas
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
