[PATCH RESEND] erofs: use Z_EROFS_LCLUSTER_TYPE_MAX to simplify switches

[Hongzhen Luo]

On 2025/2/7 15:33, Hongbo Li wrote:
>
>
> On 2025/2/7 14:41, Hongzhen Luo wrote:
>> There's no need to enumerate each type. No logic changes.
>>
>> Signed-off-by: Hongzhen Luo <hongzhen at linux.alibaba.com>
>> ---
>>   fs/erofs/zmap.c | 59 ++++++++++++++++++-------------------------------
>>   1 file changed, 22 insertions(+), 37 deletions(-)
>>
>> diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
>> index 689437e99a5a..0ee78413bfd5 100644
>> --- a/fs/erofs/zmap.c
>> +++ b/fs/erofs/zmap.c
>> @@ -265,24 +265,20 @@ static int z_erofs_extent_lookback(struct 
>> z_erofs_maprecorder *m,
>>           if (err)
>>               return err;
>>   -        switch (m->type) {
>> -        case Z_EROFS_LCLUSTER_TYPE_NONHEAD:
>> +        if (m->type == Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
>>               lookback_distance = m->delta[0];
>>               if (!lookback_distance)
>>                   goto err_bogus;
>>               continue;
>> -        case Z_EROFS_LCLUSTER_TYPE_PLAIN:
>> -        case Z_EROFS_LCLUSTER_TYPE_HEAD1:
>> -        case Z_EROFS_LCLUSTER_TYPE_HEAD2:
>> +        } else if (m->type < Z_EROFS_LCLUSTER_TYPE_MAX) {
>>               m->headtype = m->type;
>>               m->map->m_la = (lcn << lclusterbits) | m->clusterofs;
>>               return 0;
>> -        default:
>> -            erofs_err(sb, "unknown type %u @ lcn %lu of nid %llu",
>> -                  m->type, lcn, vi->nid);
>> -            DBG_BUGON(1);
>> -            return -EOPNOTSUPP;
>>           }
>> +        erofs_err(sb, "unknown type %u @ lcn %lu of nid %llu",
>> +              m->type, lcn, vi->nid);
>> +        DBG_BUGON(1);
>> +        return -EOPNOTSUPP;
>
> May be it would be easier to understand if you put the exception 
> branch at the beginning. Such as:
>
> if (m->type >= Z_EROFS_LCLUSTER_TYPE_MAX) {
>   // return -EOPNOTSUPP;
> }
>
> if (m->type == Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
>   // do something
> }
>
> // do something for other cases..
>
> This is also useful for other places. :)
>
> Thanks,
> Hongbo
>
Okay, I will send an improved version later.

Thanks,

Hongzhen

>>       }
>>   err_bogus:
>>       erofs_err(sb, "bogus lookback distance %u @ lcn %lu of nid %llu",
>> @@ -329,35 +325,28 @@ static int 
>> z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m,
>>       DBG_BUGON(lcn == initial_lcn &&
>>             m->type == Z_EROFS_LCLUSTER_TYPE_NONHEAD);
>>   -    switch (m->type) {
>> -    case Z_EROFS_LCLUSTER_TYPE_PLAIN:
>> -    case Z_EROFS_LCLUSTER_TYPE_HEAD1:
>> -    case Z_EROFS_LCLUSTER_TYPE_HEAD2:
>> +    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;
>> +    } else if (m->type < Z_EROFS_LCLUSTER_TYPE_MAX) {
>>           /*
>>            * if the 1st NONHEAD lcluster is actually PLAIN or HEAD type
>>            * rather than CBLKCNT, it's a 1 block-sized pcluster.
>>            */
>>           m->compressedblks = 1;
>> -        break;
>> -    case Z_EROFS_LCLUSTER_TYPE_NONHEAD:
>> -        if (m->delta[0] != 1)
>> -            goto err_bonus_cblkcnt;
>> -        if (m->compressedblks)
>> -            break;
>> -        fallthrough;
>> -    default:
>> -        erofs_err(sb, "cannot found CBLKCNT @ lcn %lu of nid %llu", 
>> lcn,
>> -              vi->nid);
>> -        DBG_BUGON(1);
>> -        return -EFSCORRUPTED;
>> +        goto out;
>>       }
>> +    erofs_err(sb, "cannot found CBLKCNT @ lcn %lu of nid %llu", lcn, 
>> vi->nid);
>> +    DBG_BUGON(1);
>> +    return -EFSCORRUPTED;
>>   out:
>>       m->map->m_plen = erofs_pos(sb, m->compressedblks);
>>       return 0;
>> -err_bonus_cblkcnt:
>> -    erofs_err(sb, "bogus CBLKCNT @ lcn %lu of nid %llu", lcn, vi->nid);
>> -    DBG_BUGON(1);
>> -    return -EFSCORRUPTED;
>>   }
>>     static int z_erofs_get_extent_decompressedlen(struct 
>> z_erofs_maprecorder *m)
>> @@ -386,9 +375,7 @@ static int 
>> z_erofs_get_extent_decompressedlen(struct z_erofs_maprecorder *m)
>>                   m->delta[1] = 1;
>>                   DBG_BUGON(1);
>>               }
>> -        } else if (m->type == Z_EROFS_LCLUSTER_TYPE_PLAIN ||
>> -               m->type == Z_EROFS_LCLUSTER_TYPE_HEAD1 ||
>> -               m->type == Z_EROFS_LCLUSTER_TYPE_HEAD2) {
>> +        } else if (m->type < Z_EROFS_LCLUSTER_TYPE_MAX) {
>>               if (lcn != headlcn)
>>                   break;    /* ends at the next HEAD lcluster */
>>               m->delta[1] = 1;
>> @@ -452,8 +439,7 @@ static int z_erofs_do_map_blocks(struct inode 
>> *inode,
>>           }
>>           /* m.lcn should be >= 1 if endoff < m.clusterofs */
>>           if (!m.lcn) {
>> -            erofs_err(inode->i_sb,
>> -                  "invalid logical cluster 0 at nid %llu",
>> +            erofs_err(inode->i_sb, "invalid logical cluster 0 at nid 
>> %llu",
>>                     vi->nid);
>>               err = -EFSCORRUPTED;
>>               goto unmap_out;
>> @@ -469,8 +455,7 @@ static int z_erofs_do_map_blocks(struct inode 
>> *inode,
>>               goto unmap_out;
>>           break;
>>       default:
>> -        erofs_err(inode->i_sb,
>> -              "unknown type %u @ offset %llu of nid %llu",
>> +        erofs_err(inode->i_sb, "unknown type %u @ offset %llu of nid 
>> %llu",
>>                 m.type, ofs, vi->nid);
>>           err = -EOPNOTSUPP;
>>           goto unmap_out;
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/linux-erofs/attachments/20250207/48c13481/attachment.htm>