On Tue, Apr 16, 2024 at 10:28:42PM +0200, Andrey Albershteyn wrote:
> Make flist_find_type() more readable by unloading field type
> matching to the helper.
>
> Signed-off-by: Andrey Albershteyn <aalbersh@xxxxxxxxxx>
> ---
> db/flist.c | 59 ++++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 37 insertions(+), 22 deletions(-)
>
> diff --git a/db/flist.c b/db/flist.c
> index 0a6cc5fcee43..18052a744a65 100644
> --- a/db/flist.c
> +++ b/db/flist.c
> @@ -400,6 +400,40 @@ flist_split(
> return v;
> }
>
> +flist_t *
> +flist_field_match(
Is this function going to be used outside of this module, or should it
be declared static?
> + const field_t *field,
> + fldt_t type,
> + void *obj,
> + int startoff)
> +{
> + flist_t *fl;
> + int count;
> + const ftattr_t *fa;
> +
> + fl = flist_make(field->name);
> + fl->fld = field;
> + if (field->ftyp == type)
> + return fl;
> + count = fcount(field, obj, startoff);
> + if (!count)
> + goto out;
> + fa = &ftattrtab[field->ftyp];
> + if (fa->subfld) {
You could do:
if (!fa->subfld)
goto out;
nfl = flist_find_ftyp(...);
to reduce the indenting here. But I think you were trying to make
before and after as identical as possible, right?
> + flist_t *nfl;
> +
> + nfl = flist_find_ftyp(fa->subfld, type, obj, startoff);
> + if (nfl) {
> + fl->child = nfl;
> + return fl;
> + }
> + }
> +
> +out:
> + flist_free(fl);
> + return NULL;
> +}
> +
> /*
> * Given a set of fields, scan for a field of the given type.
> * Return an flist leading to the first found field
> @@ -413,33 +447,14 @@ flist_find_ftyp(
> void *obj,
> int startoff)
> {
> - flist_t *fl;
> const field_t *f;
> - int count;
> - const ftattr_t *fa;
> + flist_t *fl;
>
> for (f = fields; f->name; f++) {
> - fl = flist_make(f->name);
> - fl->fld = f;
> - if (f->ftyp == type)
> + if ((fl = flist_field_match(f, type, obj, startoff)) != NULL)
> return fl;
Normally these days we expand assign-and-check when not in a loop
control test:
fl = flist_field_match(...);
if (fl)
return fl;
But those last two comments merely reflect my style preference; the
question I care most about is the one about the static attribute.
--D
> - count = fcount(f, obj, startoff);
> - if (!count) {
> - flist_free(fl);
> - continue;
> - }
> - fa = &ftattrtab[f->ftyp];
> - if (fa->subfld) {
> - flist_t *nfl;
> -
> - nfl = flist_find_ftyp(fa->subfld, type, obj, startoff);
> - if (nfl) {
> - fl->child = nfl;
> - return fl;
> - }
> - }
> - flist_free(fl);
> }
> +
> return NULL;
> }
>
> --
> 2.42.0
>
>
