Re: [PATCH v5] erofs: use Z_EROFS_LCLUSTER_TYPE_MAX to simplify switches

[Gao Xiang]

On 2025/3/17 14:42, Chao Yu wrote:
> On 3/17/25 14:15, Gao Xiang wrote:
> > Hi Chao,
> >
> > On 2025/3/17 14:03, Chao Yu wrote:
> > > On 3/17/25 01:17, Gao Xiang wrote:
> > > > Hi Chao,
> >
> > ...
> >
> > > > Previously, it was useful before Z_EROFS_LCLUSTER_TYPE_HEAD2 was
> > > > introduced, but the `default:` case is already deadcode now.
> > >
> > > Xiang, thanks for the explanation.
> > >
> > > So seems it can happen when mounting last image w/ old kernel which can not
> > > support newly introduced Z_EROFS_LCLUSTER_TYPE_* type, then it makes sense to
> > > return EOPNOTSUPP.
> >
> > Yeah.
> >
> > > > > Btw, we'd better to do sanity check for m->type in z_erofs_load_full_lcluster(),
> > > > > then we can treat m->type as reliable variable later.
> > > > >
> > > > >        advise = le16_to_cpu(di->di_advise);
> > > > >        m->type = advise & Z_EROFS_LI_LCLUSTER_TYPE_MASK;
> > > > >        if (m->type >= Z_EROFS_LCLUSTER_TYPE_MAX) {
> > > >
> > > > It's always false here.
> > >
> > > So, what do you think of this?
> > >
> > >   From af584b2eacd468f145e9ee31ccdeedb7355d5afd Mon Sep 17 00:00:00 2001
> > > From: Chao Yu <chao@xxxxxxxxxx>
> > > Date: Mon, 17 Mar 2025 13:57:55 +0800
> > > Subject: [PATCH] erofs: remove dead codes for cleanup
> > >
> > > z_erofs_extent_lookback() and z_erofs_get_extent_decompressedlen() tries
> > > to do sanity check on m->type, however their caller z_erofs_map_blocks_fo()
> > > has already checked that, so let's remove those dead codes.
> >
> > z_erofs_extent_lookback() will (lookback) read new lcn in
> > z_erofs_load_lcluster_from_disk() so it won't be covered by
> > the original z_erofs_map_blocks_fo().
>
> Xiang,
>
> Oh, I see, changed here:
>
> - z_erofs_extent_lookback
>   - z_erofs_load_lcluster_from_disk
>    - z_erofs_load_full_lcluster
>    : m->type = advise & Z_EROFS_LI_LCLUSTER_TYPE_MASK;
>   - z_erofs_load_compact_lcluster
>   : m->type = type;

Yeah, we'd better to move all checks into
z_erofs_load_lcluster_from_disk() later.

> > I think this check can be resolved in
> > z_erofs_load_lcluster_from_disk() instead but maybe address
> > for the next cycle? since there are already enough features
> > for this cycle and I have to make sure no major issues....
>
> Yeah, it's fine to check the cleanup later, let's keep focusing
> on improving patches in dev now.

Yes.

Thanks,
Gao Xiang

> Thanks,
>
> > Thanks,
> > Gao Xiang