On 7/8/25 10:58, Gao Xiang wrote:
> Hi Chao,
>
> On 2025/7/7 16:47, Chao Yu wrote:
>> All below functions will do sanity check on m->type, let's move sanity
>> check to z_erofs_load_compact_lcluster() for cleanup.
>> - z_erofs_map_blocks_fo
>> - z_erofs_get_extent_compressedlen
>> - z_erofs_get_extent_decompressedlen
>> - z_erofs_extent_lookback
>>
>> Signed-off-by: Chao Yu <c...@kernel.org>
> How about appending the following diff to tidy up the code
> a bit further (avoid `goto map_block` and `goto out`)?
Xiang,
Looks good to me, will append the diff in the patch.
Thanks,
>
>
> fs/erofs/zmap.c | 67 +++++++++++++++++++++++--------------------------
> 1 file changed, 31 insertions(+), 36 deletions(-)
>
> diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
> index e530b152e14e..431199452542 100644
> --- a/fs/erofs/zmap.c
> +++ b/fs/erofs/zmap.c
> @@ -327,21 +327,18 @@ static int z_erofs_get_extent_compressedlen(struct
> z_erofs_maprecorder *m,
> DBG_BUGON(lcn == initial_lcn &&
> m->type == Z_EROFS_LCLUSTER_TYPE_NONHEAD);
>
> - if (m->type == Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
> - if (m->delta[0] != 1) {
> - erofs_err(sb, "bogus CBLKCNT @ lcn %lu of nid %llu", lcn,
> vi->nid);
> - DBG_BUGON(1);
> - return -EFSCORRUPTED;
> - }
> - if (m->compressedblks)
> - goto out;
> + if (m->type == Z_EROFS_LCLUSTER_TYPE_NONHEAD && m->delta[0] != 1) {
> + erofs_err(sb, "bogus CBLKCNT @ lcn %lu of nid %llu", lcn, vi->nid);
> + DBG_BUGON(1);
> + return -EFSCORRUPTED;
> }
>
> /*
> * if the 1st NONHEAD lcluster is actually PLAIN or HEAD type rather
> * than CBLKCNT, it's a 1 block-sized pcluster.
> */
> - m->compressedblks = 1;
> + if (m->type != Z_EROFS_LCLUSTER_TYPE_NONHEAD || !m->compressedblks)
> + m->compressedblks = 1;
> out:
> m->map->m_plen = erofs_pos(sb, m->compressedblks);
> return 0;
> @@ -422,36 +419,34 @@ static int z_erofs_map_blocks_fo(struct inode *inode,
> map->m_flags = EROFS_MAP_MAPPED | EROFS_MAP_ENCODED;
> end = (m.lcn + 1ULL) << lclusterbits;
>
> - if (m.type != Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
> - if (endoff >= m.clusterofs) {
> - m.headtype = m.type;
> - map->m_la = (m.lcn << lclusterbits) | m.clusterofs;
> - /*
> - * For ztailpacking files, in order to inline data more
> - * effectively, special EOF lclusters are now supported
> - * which can have three parts at most.
> - */
> - if (ztailpacking && end > inode->i_size)
> - end = inode->i_size;
> - goto map_block;
> + if (m.type != Z_EROFS_LCLUSTER_TYPE_NONHEAD && endoff >= m.clusterofs) {
> + m.headtype = m.type;
> + map->m_la = (m.lcn << lclusterbits) | m.clusterofs;
> + /*
> + * For ztailpacking files, in order to inline data more
> + * effectively, special EOF lclusters are now supported
> + * which can have three parts at most.
> + */
> + if (ztailpacking && end > inode->i_size)
> + end = inode->i_size;
> + } else {
> + if (m.type != Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
> + /* m.lcn should be >= 1 if endoff < m.clusterofs */
> + if (!m.lcn) {
> + erofs_err(sb, "invalid logical cluster 0 at nid %llu",
> + vi->nid);
> + err = -EFSCORRUPTED;
> + goto unmap_out;
> + }
> + end = (m.lcn << lclusterbits) | m.clusterofs;
> + map->m_flags |= EROFS_MAP_FULL_MAPPED;
> + m.delta[0] = 1;
> }
> - /* m.lcn should be >= 1 if endoff < m.clusterofs */
> - if (!m.lcn) {
> - erofs_err(sb, "invalid logical cluster 0 at nid %llu",
> - vi->nid);
> - err = -EFSCORRUPTED;
> + /* get the corresponding first chunk */
> + err = z_erofs_extent_lookback(&m, m.delta[0]);
> + if (err)
> goto unmap_out;
> - }
> - end = (m.lcn << lclusterbits) | m.clusterofs;
> - map->m_flags |= EROFS_MAP_FULL_MAPPED;
> - m.delta[0] = 1;
> }
> -
> - /* get the corresponding first chunk */
> - err = z_erofs_extent_lookback(&m, m.delta[0]);
> - if (err)
> - goto unmap_out;
> -map_block:
> if (m.partialref)
> map->m_flags |= EROFS_MAP_PARTIAL_REF;
> map->m_llen = end - map->m_la;
