fs/erofs/zmap.c | 60 +++++++++++++++++++------------------------------ 1 file changed, 23 insertions(+), 37 deletions(-)
There's no need to enumerate each type. No logic changes.
Signed-off-by: Hongzhen Luo <hongzhen@linux.alibaba.com>
---
v3: Code cleanup, remove logically inequivalent changes.
v2: https://lore.kernel.org/all/20250207080829.2405528-1-hongzhen@linux.alibaba.com/
v1: https://lore.kernel.org/all/20250207064135.2249529-1-hongzhen@linux.alibaba.com/
---
fs/erofs/zmap.c | 60 +++++++++++++++++++------------------------------
1 file changed, 23 insertions(+), 37 deletions(-)
diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
index 689437e99a5a..7dba1573498b 100644
--- a/fs/erofs/zmap.c
+++ b/fs/erofs/zmap.c
@@ -265,23 +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_MAX) {
+ erofs_err(sb, "unknown type %u @ lcn %lu of nid %llu",
+ m->type, lcn, vi->nid);
+ DBG_BUGON(1);
+ return -EOPNOTSUPP;
+ } else 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 {
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 +326,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 +376,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 +440,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 +456,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:50, Hongzhen Luo wrote:
> There's no need to enumerate each type. No logic changes.
>
> Signed-off-by: Hongzhen Luo <hongzhen@linux.alibaba.com>
> ---> v3: Code cleanup, remove logically inequivalent changes.
> v2: https://lore.kernel.org/all/20250207080829.2405528-1-hongzhen@linux.alibaba.com/
> v1: https://lore.kernel.org/all/20250207064135.2249529-1-hongzhen@linux.alibaba.com/
> ---
> fs/erofs/zmap.c | 60 +++++++++++++++++++------------------------------
> 1 file changed, 23 insertions(+), 37 deletions(-)
>
> diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
> index 689437e99a5a..7dba1573498b 100644
> --- a/fs/erofs/zmap.c
> +++ b/fs/erofs/zmap.c
> @@ -265,23 +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_MAX) {
> + erofs_err(sb, "unknown type %u @ lcn %lu of nid %llu",
> + m->type, lcn, vi->nid);
> + DBG_BUGON(1);
> + return -EOPNOTSUPP;
> + } else if (m->type == Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
> lookback_distance = m->delta[0];
> if (!lookback_distance)
> goto err_bogus;'
Maybe just kill `goto err_bogus;` too, like:
if (!lookback_distance) {
erofs_err(sb, "bogus lookback distance %u @ lcn %lu of nid %llu",
lookback_distance, m->lcn, vi->nid);
DBG_BUGON(1);
return -EFSCORRUPTED;
}
Otherwise it looks good to me.
Thanks,
Gao Xiang
On 2025/2/7 17:14, Gao Xiang wrote:
>
>
> On 2025/2/7 16:50, Hongzhen Luo wrote:
>> There's no need to enumerate each type. No logic changes.
>>
>> Signed-off-by: Hongzhen Luo <hongzhen@linux.alibaba.com>
>> ---> v3: Code cleanup, remove logically inequivalent changes.
>> v2: https://lore.kernel.org/all/20250207080829.2405528-1-hongzhen@linux.alibaba.com/
>> v1: https://lore.kernel.org/all/20250207064135.2249529-1-hongzhen@linux.alibaba.com/
>> ---
>> fs/erofs/zmap.c | 60 +++++++++++++++++++------------------------------
>> 1 file changed, 23 insertions(+), 37 deletions(-)
>>
>> diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
>> index 689437e99a5a..7dba1573498b 100644
>> --- a/fs/erofs/zmap.c
>> +++ b/fs/erofs/zmap.c
>> @@ -265,23 +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_MAX) {
>> + erofs_err(sb, "unknown type %u @ lcn %lu of nid %llu",
>> + m->type, lcn, vi->nid);
>> + DBG_BUGON(1);
>> + return -EOPNOTSUPP;
>> + } else if (m->type == Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
>> lookback_distance = m->delta[0];
>> if (!lookback_distance)
>> goto err_bogus;'
>
> Maybe just kill `goto err_bogus;` too, like:
> if (!lookback_distance) {
> erofs_err(sb, "bogus lookback distance %u @ lcn %lu of nid %llu",
> lookback_distance, m->lcn, vi->nid);
> DBG_BUGON(1);
> return -EFSCORRUPTED;
> }
>
> Otherwise it looks good to me.
Sorry, I was wrong. I was missing the part of the end of
function, how about the following diff?
diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
index 7dba1573498b..d278ebd60281 100644
--- a/fs/erofs/zmap.c
+++ b/fs/erofs/zmap.c
@@ -273,7 +273,7 @@ static int z_erofs_extent_lookback(struct z_erofs_maprecorder *m,
} else if (m->type == Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
lookback_distance = m->delta[0];
if (!lookback_distance)
- goto err_bogus;
+ break;
continue;
} else {
m->headtype = m->type;
@@ -281,7 +281,6 @@ static int z_erofs_extent_lookback(struct z_erofs_maprecorder *m,
return 0;
}
}
-err_bogus:
erofs_err(sb, "bogus lookback distance %u @ lcn %lu of nid %llu",
lookback_distance, m->lcn, vi->nid);
DBG_BUGON(1);
Thanks,
Gao Xiang
On 2025/2/10 10:59, Gao Xiang wrote:
>
>
> On 2025/2/7 17:14, Gao Xiang wrote:
>>
>>
>> On 2025/2/7 16:50, Hongzhen Luo wrote:
>>> There's no need to enumerate each type. No logic changes.
>>>
>>> Signed-off-by: Hongzhen Luo <hongzhen@linux.alibaba.com>
>>> ---> v3: Code cleanup, remove logically inequivalent changes.
>>> v2:
>>> https://lore.kernel.org/all/20250207080829.2405528-1-hongzhen@linux.alibaba.com/
>>> v1:
>>> https://lore.kernel.org/all/20250207064135.2249529-1-hongzhen@linux.alibaba.com/
>>> ---
>>> fs/erofs/zmap.c | 60
>>> +++++++++++++++++++------------------------------
>>> 1 file changed, 23 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
>>> index 689437e99a5a..7dba1573498b 100644
>>> --- a/fs/erofs/zmap.c
>>> +++ b/fs/erofs/zmap.c
>>> @@ -265,23 +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_MAX) {
>>> + erofs_err(sb, "unknown type %u @ lcn %lu of nid %llu",
>>> + m->type, lcn, vi->nid);
>>> + DBG_BUGON(1);
>>> + return -EOPNOTSUPP;
>>> + } else if (m->type == Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
>>> lookback_distance = m->delta[0];
>>> if (!lookback_distance)
>>> goto err_bogus;'
>>
>> Maybe just kill `goto err_bogus;` too, like:
>> if (!lookback_distance) {
>> erofs_err(sb, "bogus lookback
>> distance %u @ lcn %lu of nid %llu",
>> lookback_distance, m->lcn,
>> vi->nid);
>> DBG_BUGON(1);
>> return -EFSCORRUPTED;
>> }
>>
>> Otherwise it looks good to me.
>
> Sorry, I was wrong. I was missing the part of the end of
> function, how about the following diff?
>
> diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
> index 7dba1573498b..d278ebd60281 100644
> --- a/fs/erofs/zmap.c
> +++ b/fs/erofs/zmap.c
> @@ -273,7 +273,7 @@ static int z_erofs_extent_lookback(struct
> z_erofs_maprecorder *m,
> } else if (m->type == Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
> lookback_distance = m->delta[0];
> if (!lookback_distance)
> - goto err_bogus;
> + break;
> continue;
> } else {
> m->headtype = m->type;
> @@ -281,7 +281,6 @@ static int z_erofs_extent_lookback(struct
> z_erofs_maprecorder *m,
> return 0;
> }
> }
> -err_bogus:
> erofs_err(sb, "bogus lookback distance %u @ lcn %lu of nid %llu",
> lookback_distance, m->lcn, vi->nid);
> DBG_BUGON(1);
>
> Thanks,
> Gao Xiang
Sure, I will send an updated version later.
Thanks,
Hongzhen Luo
On 2025/2/7 17:14, Gao Xiang wrote:
>
>
> On 2025/2/7 16:50, Hongzhen Luo wrote:
>> There's no need to enumerate each type. No logic changes.
>>
>> Signed-off-by: Hongzhen Luo <hongzhen@linux.alibaba.com>
>> ---> v3: Code cleanup, remove logically inequivalent changes.
>> v2:
>> https://lore.kernel.org/all/20250207080829.2405528-1-hongzhen@linux.alibaba.com/
>> v1:
>> https://lore.kernel.org/all/20250207064135.2249529-1-hongzhen@linux.alibaba.com/
>> ---
>> fs/erofs/zmap.c | 60 +++++++++++++++++++------------------------------
>> 1 file changed, 23 insertions(+), 37 deletions(-)
>>
>> diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
>> index 689437e99a5a..7dba1573498b 100644
>> --- a/fs/erofs/zmap.c
>> +++ b/fs/erofs/zmap.c
>> @@ -265,23 +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_MAX) {
>> + erofs_err(sb, "unknown type %u @ lcn %lu of nid %llu",
>> + m->type, lcn, vi->nid);
>> + DBG_BUGON(1);
>> + return -EOPNOTSUPP;
>> + } else if (m->type == Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
>> lookback_distance = m->delta[0];
>> if (!lookback_distance)
>> goto err_bogus;'
>
> Maybe just kill `goto err_bogus;` too, like:
> if (!lookback_distance) {
> erofs_err(sb, "bogus lookback distance
> %u @ lcn %lu of nid %llu",
> lookback_distance, m->lcn,
> vi->nid);
> DBG_BUGON(1);
> return -EFSCORRUPTED;
> }
>
> Otherwise it looks good to me.
>
> Thanks,
> Gao Xiang
Sure, I will send an updated version later.
Thanks,
Hongzhen
© 2016 - 2026 Red Hat, Inc.