fs/erofs/zmap.c | 65 +++++++++++++++++++++---------------------------- 1 file changed, 28 insertions(+), 37 deletions(-)
There's no need to enumerate each type. No logic changes.
Signed-off-by: Hongzhen Luo <hongzhen@linux.alibaba.com>
---
Changes since v1: Put the exception branch at the beginning.
v1: https://lore.kernel.org/all/20250207064135.2249529-1-hongzhen@linux.alibaba.com/
---
fs/erofs/zmap.c | 65 +++++++++++++++++++++----------------------------
1 file changed, 28 insertions(+), 37 deletions(-)
diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
index 689437e99a5a..f65c76991e02 100644
--- a/fs/erofs/zmap.c
+++ b/fs/erofs/zmap.c
@@ -265,23 +265,22 @@ 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_MAX) {
+ erofs_err(sb, "unknown type %u @ lcn %lu of nid %llu",
+ m->type, lcn, vi->nid);
+ DBG_BUGON(1);
+ return -EOPNOTSUPP;
+ }
+
+ 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;
}
}
err_bogus:
@@ -329,35 +328,31 @@ 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_MAX) {
+ erofs_err(sb, "cannot found CBLKCNT @ lcn %lu of nid %llu", lcn, vi->nid);
+ DBG_BUGON(1);
+ return -EFSCORRUPTED;
+ }
+
+ 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;
}
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 +381,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 +445,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 +461,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;
--
2.43.5
On 2025/2/7 16:08, Hongzhen Luo wrote:
> There's no need to enumerate each type. No logic changes.
>
> Signed-off-by: Hongzhen Luo <hongzhen@linux.alibaba.com>
> ---
> Changes since v1: Put the exception branch at the beginning.
>
> v1: https://lore.kernel.org/all/20250207064135.2249529-1-hongzhen@linux.alibaba.com/
> ---
> fs/erofs/zmap.c | 65 +++++++++++++++++++++----------------------------
> 1 file changed, 28 insertions(+), 37 deletions(-)
>
> diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
> index 689437e99a5a..f65c76991e02 100644
> --- a/fs/erofs/zmap.c
> +++ b/fs/erofs/zmap.c
> @@ -265,23 +265,22 @@ 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_MAX) {
> + erofs_err(sb, "unknown type %u @ lcn %lu of nid %llu",
> + m->type, lcn, vi->nid);
> + DBG_BUGON(1);
> + return -EOPNOTSUPP;
> + }
> +
> + 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) {
Just `} else {` here?
> 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;
> }
> }
> err_bogus:
> @@ -329,35 +328,31 @@ 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_MAX) {
> + erofs_err(sb, "cannot found CBLKCNT @ lcn %lu of nid %llu", lcn, vi->nid);
> + DBG_BUGON(1);
> + return -EFSCORRUPTED;
> + }
No, I don't think it's equivalent, please use
the previous version for this part instead.
Thanks,
Gao Xiang
On 2025/2/7 16:29, Gao Xiang wrote:
>
>
> On 2025/2/7 16:08, Hongzhen Luo wrote:
>> There's no need to enumerate each type. No logic changes.
>>
>> Signed-off-by: Hongzhen Luo <hongzhen@linux.alibaba.com>
>> ---
>> Changes since v1: Put the exception branch at the beginning.
>>
>> v1:
>> https://lore.kernel.org/all/20250207064135.2249529-1-hongzhen@linux.alibaba.com/
>> ---
>> fs/erofs/zmap.c | 65 +++++++++++++++++++++----------------------------
>> 1 file changed, 28 insertions(+), 37 deletions(-)
>>
>> diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
>> index 689437e99a5a..f65c76991e02 100644
>> --- a/fs/erofs/zmap.c
>> +++ b/fs/erofs/zmap.c
>> @@ -265,23 +265,22 @@ 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_MAX) {
>> + erofs_err(sb, "unknown type %u @ lcn %lu of nid %llu",
>> + m->type, lcn, vi->nid);
>> + DBG_BUGON(1);
>> + return -EOPNOTSUPP;
>> + }
>> +
>> + 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) {
>
>
> Just `} else {` here?
>
>> 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;
>> }
>> }
>> err_bogus:
>> @@ -329,35 +328,31 @@ 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_MAX) {
>> + erofs_err(sb, "cannot found CBLKCNT @ lcn %lu of nid %llu",
>> lcn, vi->nid);
>> + DBG_BUGON(1);
>> + return -EFSCORRUPTED;
>> + }
>
> No, I don't think it's equivalent, please use
> the previous version for this part instead.
I think this version doesn't consider the fallthrough case. May be just
use the goto statement can keep equivalent.
Thanks,
Hongbo
>
> Thanks,
> Gao Xiang
Hi Hongbo,
On 2025/2/7 16:57, Hongbo Li via Linux-erofs wrote:
>
...
>>> @@ -329,35 +328,31 @@ 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_MAX) {
>>> + erofs_err(sb, "cannot found CBLKCNT @ lcn %lu of nid %llu", lcn, vi->nid);
>>> + DBG_BUGON(1);
>>> + return -EFSCORRUPTED;
>>> + }
>>
>> No, I don't think it's equivalent, please use
>> the previous version for this part instead.
>
> I think this version doesn't consider the fallthrough case. May be just use the goto statement can keep equivalent.
For this particular part, I tend to use the first
version since it's already simple enough anyway.
Thanks,
Gao Xiang
>
> Thanks,
> Hongbo
>
>>
>> Thanks,
>> Gao Xiang
© 2016 - 2026 Red Hat, Inc.