Re: [PATCH 2/2] erofs-utils: refactor: remove end argument from erofs_mapbh

["胡 玮文" <huww98@outlook.com>]

Hi Xiang,

If I understand it correctly, when ‘end == false’, the last buffer block will not get its ‘blkaddr’. I can’t see why we need this, but I might be missing something. And every invocation of this function just pass ‘true’ to this argument. I’m intended to remove some dead code which is never invoked.

> 在 2021年1月2日，13:15，Gao Xiang <hsiangkao@redhat.com> 写道：
> 
> Hi Weiwen,
> 
>> On Fri, Jan 01, 2021 at 05:11:58PM +0800, 胡玮文 wrote:
>> Signed-off-by: Hu Weiwen <huww98@outlook.com>
> 
> It seems that it drops the needed argument `end'.
> 
> The original purpose of that is to get the beginning/end blkaddr of a buffer block
> in preparation for later use, some specific reasons to get rid of it?
> (also it only drops one extra line considering the diffstat....)

That’s because I already assumed ‘end == true’ and rewritten this function in [PATCH 1/2]

Thanks for your reply.

Hu Weiwen

> 
> Thanks,
> Gao Xiang
> 
>> ---
>> include/erofs/cache.h |  2 +-
>> lib/cache.c           |  3 +--
>> lib/compress.c        |  2 +-
>> lib/inode.c           | 10 +++++-----
>> lib/xattr.c           |  2 +-
>> mkfs/main.c           |  2 +-
>> 6 files changed, 10 insertions(+), 11 deletions(-)
>> 
>> diff --git a/include/erofs/cache.h b/include/erofs/cache.h
>> index 8c171f5..f8dff67 100644
>> --- a/include/erofs/cache.h
>> +++ b/include/erofs/cache.h
>> @@ -95,7 +95,7 @@ struct erofs_buffer_head *erofs_balloc(int type, erofs_off_t size,
>> struct erofs_buffer_head *erofs_battach(struct erofs_buffer_head *bh,
>>                    int type, unsigned int size);
>> 
>> -erofs_blk_t erofs_mapbh(struct erofs_buffer_block *bb, bool end);
>> +erofs_blk_t erofs_mapbh(struct erofs_buffer_block *bb);
>> bool erofs_bflush(struct erofs_buffer_block *bb);
>> 
>> void erofs_bdrop(struct erofs_buffer_head *bh, bool tryrevoke);
>> diff --git a/lib/cache.c b/lib/cache.c
>> index 3412a0b..9765cfd 100644
>> --- a/lib/cache.c
>> +++ b/lib/cache.c
>> @@ -329,9 +329,8 @@ static erofs_blk_t __erofs_mapbh(struct erofs_buffer_block *bb)
>>    return blkaddr;
>> }
>> 
>> -erofs_blk_t erofs_mapbh(struct erofs_buffer_block *bb, bool end)
>> +erofs_blk_t erofs_mapbh(struct erofs_buffer_block *bb)
>> {
>> -    DBG_BUGON(!end);
>>    struct erofs_buffer_block *t = last_mapped_block;
>>    while (1) {
>>        t = list_next_entry(t, list);
>> diff --git a/lib/compress.c b/lib/compress.c
>> index 86db940..2b1f93c 100644
>> --- a/lib/compress.c
>> +++ b/lib/compress.c
>> @@ -416,7 +416,7 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
>> 
>>    memset(compressmeta, 0, Z_EROFS_LEGACY_MAP_HEADER_SIZE);
>> 
>> -    blkaddr = erofs_mapbh(bh->block, true);    /* start_blkaddr */
>> +    blkaddr = erofs_mapbh(bh->block);    /* start_blkaddr */
>>    ctx.blkaddr = blkaddr;
>>    ctx.metacur = compressmeta + Z_EROFS_LEGACY_MAP_HEADER_SIZE;
>>    ctx.head = ctx.tail = 0;
>> diff --git a/lib/inode.c b/lib/inode.c
>> index 3d634fc..60666bb 100644
>> --- a/lib/inode.c
>> +++ b/lib/inode.c
>> @@ -150,7 +150,7 @@ static int __allocate_inode_bh_data(struct erofs_inode *inode,
>>    inode->bh_data = bh;
>> 
>>    /* get blkaddr of the bh */
>> -    ret = erofs_mapbh(bh->block, true);
>> +    ret = erofs_mapbh(bh->block);
>>    DBG_BUGON(ret < 0);
>> 
>>    /* write blocks except for the tail-end block */
>> @@ -524,7 +524,7 @@ int erofs_prepare_tail_block(struct erofs_inode *inode)
>>        bh->op = &erofs_skip_write_bhops;
>> 
>>        /* get blkaddr of bh */
>> -        ret = erofs_mapbh(bh->block, true);
>> +        ret = erofs_mapbh(bh->block);
>>        DBG_BUGON(ret < 0);
>>        inode->u.i_blkaddr = bh->block->blkaddr;
>> 
>> @@ -634,7 +634,7 @@ int erofs_write_tail_end(struct erofs_inode *inode)
>>        int ret;
>>        erofs_off_t pos;
>> 
>> -        erofs_mapbh(bh->block, true);
>> +        erofs_mapbh(bh->block);
>>        pos = erofs_btell(bh, true) - EROFS_BLKSIZ;
>>        ret = dev_write(inode->idata, pos, inode->idata_size);
>>        if (ret)
>> @@ -871,7 +871,7 @@ void erofs_fixup_meta_blkaddr(struct erofs_inode *rootdir)
>>    struct erofs_buffer_head *const bh = rootdir->bh;
>>    erofs_off_t off, meta_offset;
>> 
>> -    erofs_mapbh(bh->block, true);
>> +    erofs_mapbh(bh->block);
>>    off = erofs_btell(bh, false);
>> 
>>    if (off > rootnid_maxoffset)
>> @@ -890,7 +890,7 @@ erofs_nid_t erofs_lookupnid(struct erofs_inode *inode)
>>    if (!bh)
>>        return inode->nid;
>> 
>> -    erofs_mapbh(bh->block, true);
>> +    erofs_mapbh(bh->block);
>>    off = erofs_btell(bh, false);
>> 
>>    meta_offset = blknr_to_addr(sbi.meta_blkaddr);
>> diff --git a/lib/xattr.c b/lib/xattr.c
>> index 49ebb9c..8b7bcb1 100644
>> --- a/lib/xattr.c
>> +++ b/lib/xattr.c
>> @@ -575,7 +575,7 @@ int erofs_build_shared_xattrs_from_path(const char *path)
>>    }
>>    bh->op = &erofs_skip_write_bhops;
>> 
>> -    erofs_mapbh(bh->block, true);
>> +    erofs_mapbh(bh->block);
>>    off = erofs_btell(bh, false);
>> 
>>    sbi.xattr_blkaddr = off / EROFS_BLKSIZ;
>> diff --git a/mkfs/main.c b/mkfs/main.c
>> index c63b274..1c23560 100644
>> --- a/mkfs/main.c
>> +++ b/mkfs/main.c
>> @@ -304,7 +304,7 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
>>        round_up(EROFS_SUPER_END, EROFS_BLKSIZ);
>>    char *buf;
>> 
>> -    *blocks         = erofs_mapbh(NULL, true);
>> +    *blocks         = erofs_mapbh(NULL);
>>    sb.blocks       = cpu_to_le32(*blocks);
>>    sb.root_nid     = cpu_to_le16(root_nid);
>>    memcpy(sb.uuid, sbi.uuid, sizeof(sb.uuid));
>> -- 
>> 2.30.0
>> 
>