Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Andreas Dilger <adilger@dilger.ca>
Mime-Version: 1.0 (1.0)
Subject: Re: [PATCH 1/1] e2fsck: Add percent to files and blocks feature
Date:   Wed, 26 Jul 2023 10:15:26 -0600
Message-Id: <7023297C-8D10-4903-A0E2-7ED8B8BFA043@dilger.ca>
References: <20230423082349.53474-2-megia.oscar@gmail.com>
Cc:     linux-ext4@vger.kernel.org
In-Reply-To: <20230423082349.53474-2-megia.oscar@gmail.com>
To:     =?utf-8?Q?Oscar_Megia_L=C3=B3pez?= <megia.oscar@gmail.com>
Precedence: bulk

On Apr 23, 2023, at 02:25, Oscar Megia L=C3=B3pez <megia.oscar@gmail.com> wr=
ote:
>=20
> =EF=BB=BFI need percentages to see how disk is occupied.
> Used and maximum are good, but humans work better with percentages.
>=20
> When my linux boots,
> I haven't enough time to remember numbers and calculate.
>=20
> My PC is very fast. I can only see the message for one or two seconds.
>=20
> If also I would see percentages for me would be perfect.
>=20
> I think that this feature is going to be good for everyone.
>=20
> Signed-off-by: Oscar Megia L=C3=B3pez <megia.oscar@gmail.com>
> ---
> e2fsck/unix.c | 25 +++++++++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>=20
> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> index e5b672a2..b820ca8d 100644
> --- a/e2fsck/unix.c
> +++ b/e2fsck/unix.c
> @@ -350,6 +350,8 @@ static void check_if_skip(e2fsck_t ctx)
>    int defer_check_on_battery;
>    int broken_system_clock;
>    time_t lastcheck;
> +    char percent_files[9];
> +    char percent_blocks[9];
>=20
>    if (ctx->flags & E2F_FLAG_PROBLEMS_FIXED)
>        return;
> @@ -442,14 +444,33 @@ static void check_if_skip(e2fsck_t ctx)
>        ext2fs_mark_super_dirty(fs);
>    }
>=20
> +    /* Calculate percentages */
> +    if (fs->super->s_inodes_count > 0) {
> +        snprintf(percent_files, sizeof(percent_files), " (%u%%) ",
> +        ((fs->super->s_inodes_count - fs->super->s_free_inodes_count) * 1=
00) /
> +        fs->super->s_inodes_count);
> +    } else {
> +        snprintf(percent_files, sizeof(percent_files), " ");
> +    }

Instead of snprintf() this could just be initialized at variable declaration=
 time:

        char percent_files[8] =3D "";

That avoids extra runtime overhead and is no less safe. (This is adjusted to=
 compensate
for the format change below.)

> +    if (ext2fs_blocks_count(fs->super) > 0) {
> +        snprintf(percent_blocks, sizeof(percent_blocks), " (%llu%%) ",
> +        (unsigned long long) ((ext2fs_blocks_count(fs->super) -
> +        ext2fs_free_blocks_count(fs->super)) * 100) / ext2fs_blocks_count=
(fs->super));
> +    } else {
> +        snprintf(percent_blocks, sizeof(percent_blocks), " ");
> +    }

This could similarly be set at initialization:

        char percent_blocks[8] =3D "";

>    /* Print the summary message when we're skipping a full check */
> -    log_out(ctx, _("%s: clean, %u/%u files, %llu/%llu blocks"),
> +    log_out(ctx, _("%s: clean, %u/%u%sfiles, %llu/%llu%sblocks"),

This would be more readable if it left one space after each "%s" and then di=
dn't
include the trailing space in each string:

    log_out(ctx, _("%s: clean, %u/%u%s files, %llu/%llu%s blocks"),

Cheers, Andreas

>        ctx->device_name,
>        fs->super->s_inodes_count - fs->super->s_free_inodes_count,
>        fs->super->s_inodes_count,
> +        percent_files,
>        (unsigned long long) ext2fs_blocks_count(fs->super) -
>        ext2fs_free_blocks_count(fs->super),
> -        (unsigned long long) ext2fs_blocks_count(fs->super));
> +        (unsigned long long) ext2fs_blocks_count(fs->super),
> +        percent_blocks);
>    next_check =3D 100000;
>    if (fs->super->s_max_mnt_count > 0) {
>        next_check =3D fs->super->s_max_mnt_count - fs->super->s_mnt_count;=

> --=20
> 2.40.0
>=20