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;
From:   =?utf-8?Q?Oscar_Megia_L=C3=B3pez?= <megia.oscar@gmail.com>
To:     Andreas Dilger <adilger@dilger.ca>
Cc:     linux-ext4@vger.kernel.org
Subject: Re: [PATCH 1/1] e2fsck: Add percent to files and blocks feature
References: <20230423082349.53474-2-megia.oscar@gmail.com>
        <7023297C-8D10-4903-A0E2-7ED8B8BFA043@dilger.ca>
Date:   Thu, 27 Jul 2023 09:15:14 +0200
In-Reply-To: <7023297C-8D10-4903-A0E2-7ED8B8BFA043@dilger.ca> (Andreas
        Dilger's message of "Wed, 26 Jul 2023 10:15:26 -0600")
Message-ID: <878rb17rlp.fsf@gmail.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Andreas Dilger <adilger@dilger.ca> writes:

> On Apr 23, 2023, at 02:25, Oscar Megia L=C3=B3pez <megia.oscar@gmail.com>=
 wrote:
>>=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) *=
 100) /
>> +        fs->super->s_inodes_count);
>> +    } else {
>> +        snprintf(percent_files, sizeof(percent_files), " ");
>> +    }
>
> Instead of snprintf() this could just be initialized at variable declarat=
ion 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.)
>

Thanks for your advice, I will apply it.

>> +    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_cou=
nt(fs->super));
>> +    } else {
>> +        snprintf(percent_blocks, sizeof(percent_blocks), " ");
>> +    }
>
> This could similarly be set at initialization:
>
>         char percent_blocks[8] =3D "";
>

Thanks for your advice, I will apply it.

>>    /* 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=
 didn't
> include the trailing space in each string:
>
>     log_out(ctx, _("%s: clean, %u/%u%s files, %llu/%llu%s blocks"),
>

You are right. I will apply it.

> Cheers, Andreas
>

Very thanks Andreas for your advices and your time.

I will apply these changes to my patch.

Regards
Oscar

>>        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_coun=
t;
>> --=20
>> 2.40.0
>>=20