Re: [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 <hongz...@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;