linux-ext4 - Re: [PATCH 1/7] e2fsprogs: Add discard function into struct_io

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 18 Nov 2010 10:52:23 +0100 (CET)
From:	Lukas Czerner <lczerner@...hat.com>
To:	Eric Sandeen <sandeen@...hat.com>
cc:	Lukas Czerner <lczerner@...hat.com>, linux-ext4@...r.kernel.org,
	tytso@....edu, adilger@...ger.ca
Subject: Re: [PATCH 1/7] e2fsprogs: Add discard function into
 struct_io_manager

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html