On Tue, Jul 12, 2016 at 12:02:48AM +0100, Luis de Bethencourt wrote:
> befs_read_datastream() is used to read the inode from the disk, off is
> meant to provide the offset of the data in the buffer head. But the only
> function using this argument already knows the starting offset of the node,
> so this argument isn't needed.
>
> Signed-off-by: Luis de Bethencourt <luisbg@xxxxxxxxxxxxxxx>
> ---
> Hi,
>
> I know we are in release candidate 7 and maintainers are busy with important
> bugs and regressions. Just sending this now so it is in the queue when the
> merge window opens in two weeks.
>
> befs_bt_read_node() is the only case where befs_read_datastream() was called
> with an off pointer, the rest had NULL.
>
> befs_read_datastream() effectively did:
> block = pos >> BEFS_SB(sb)->block_shift;
> *off = pos - (block << BEFS_SB(sb)->block_shift);
>
> Since we only use it for inodes, pos above is either 0 or 1204, the node size
> in BeFS by design. That shifted makes block equal 0. So off always ends up
> being the same as pos. We can use this directly in befs_bt_read_node().
>
> Thank for the reviews,
> Luis
>
> fs/befs/btree.c | 8 +++-----
> fs/befs/datastream.c | 10 +++-------
> fs/befs/datastream.h | 2 +-
> 3 files changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/fs/befs/btree.c b/fs/befs/btree.c
> index 307645f9..3995d58 100644
> --- a/fs/befs/btree.c
> +++ b/fs/befs/btree.c
> @@ -142,7 +142,7 @@ befs_bt_read_super(struct super_block *sb, const befs_data_stream *ds,
>
> befs_debug(sb, "---> %s", __func__);
>
> - bh = befs_read_datastream(sb, ds, 0, NULL);
> + bh = befs_read_datastream(sb, ds, 0);
>
> if (!bh) {
> befs_error(sb, "Couldn't read index header.");
> @@ -196,14 +196,12 @@ static int
> befs_bt_read_node(struct super_block *sb, const befs_data_stream *ds,
> struct befs_btree_node *node, befs_off_t node_off)
> {
> - uint off = 0;
> -
> befs_debug(sb, "---> %s", __func__);
>
> if (node->bh)
> brelse(node->bh);
>
> - node->bh = befs_read_datastream(sb, ds, node_off, &off);
> + node->bh = befs_read_datastream(sb, ds, node_off);
> if (!node->bh) {
> befs_error(sb, "%s failed to read "
> "node at %llu", __func__, node_off);
> @@ -212,7 +210,7 @@ befs_bt_read_node(struct super_block *sb, const befs_data_stream *ds,
> return BEFS_ERR;
> }
> node->od_node =
> - (befs_btree_nodehead *) ((void *) node->bh->b_data + off);
> + (befs_btree_nodehead *) ((void *) node->bh->b_data + node_off);
>
> befs_dump_index_node(sb, node->od_node);
>
> diff --git a/fs/befs/datastream.c b/fs/befs/datastream.c
> index 26cc417..3c14c84 100644
> --- a/fs/befs/datastream.c
> +++ b/fs/befs/datastream.c
> @@ -39,14 +39,12 @@ static int befs_find_brun_dblindirect(struct super_block *sb,
> * @sb: Filesystem superblock
> * @ds: datastrem to find data with
> * @pos: start of data
> - * @off: offset of data in buffer_head->b_data
> *
> - * Returns pointer to buffer_head containing data starting with offset @off,
> - * if you don't need to know offset just set @off = NULL.
> + * Returns pointer to buffer_head containing data starting from pos.
> */
> struct buffer_head *
> befs_read_datastream(struct super_block *sb, const befs_data_stream *ds,
> - befs_off_t pos, uint * off)
> + befs_off_t pos)
> {
> struct buffer_head *bh;
> befs_block_run run;
> @@ -54,8 +52,6 @@ befs_read_datastream(struct super_block *sb, const befs_data_stream *ds,
>
> befs_debug(sb, "---> %s %llu", __func__, pos);
> block = pos >> BEFS_SB(sb)->block_shift;
> - if (off)
> - *off = pos - (block << BEFS_SB(sb)->block_shift);
>
> if (befs_fblock2brun(sb, ds, block, &run) != BEFS_OK) {
> befs_error(sb, "BeFS: Error finding disk addr of block %lu",
> @@ -131,7 +127,7 @@ befs_read_lsymlink(struct super_block *sb, const befs_data_stream *ds,
> befs_debug(sb, "---> %s length: %llu", __func__, len);
>
> while (bytes_read < len) {
> - bh = befs_read_datastream(sb, ds, bytes_read, NULL);
> + bh = befs_read_datastream(sb, ds, bytes_read);
> if (!bh) {
> befs_error(sb, "BeFS: Error reading datastream block "
> "starting from %llu", bytes_read);
> diff --git a/fs/befs/datastream.h b/fs/befs/datastream.h
> index 91ba820..76e1ab5 100644
> --- a/fs/befs/datastream.h
> +++ b/fs/befs/datastream.h
> @@ -5,7 +5,7 @@
>
> struct buffer_head *befs_read_datastream(struct super_block *sb,
> const befs_data_stream *ds,
> - befs_off_t pos, uint * off);
> + befs_off_t pos);
>
> int befs_fblock2brun(struct super_block *sb, const befs_data_stream *data,
> befs_blocknr_t fblock, befs_block_run * run);
> --
> 2.5.3
>
[...]
> Since we only use it for inodes, pos above is either 0 or 1204, the node size
> in BeFS by design.
pos is the byte address that corresponds to a FS block number
> block = pos >> BEFS_SB(sb)->block_shift;
> *off = pos - (block << BEFS_SB(sb)->block_shift);
the result of the second shift is equal to pos, so *off is always set to
zero.
Am I wrong ?
best regards
--
salah
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
