Re: [PATCH 1/1] e2fsck: Add percent to files and blocks feature

["Oscar Megia López" <megia.oscar@gmail.com>]

Andreas Dilger <adilger@dilger.ca> writes:

> On Apr 23, 2023, at 02:25, Oscar Megia López <megia.oscar@gmail.com> wrote:
>> 
>> I need percentages to see how disk is occupied.
>> Used and maximum are good, but humans work better with percentages.
>> 
>> When my linux boots,
>> I haven't enough time to remember numbers and calculate.
>> 
>> My PC is very fast. I can only see the message for one or two seconds.
>> 
>> If also I would see percentages for me would be perfect.
>> 
>> I think that this feature is going to be good for everyone.
>> 
>> Signed-off-by: Oscar Megia López <megia.oscar@gmail.com>
>> ---
>> e2fsck/unix.c | 25 +++++++++++++++++++++++--
>> 1 file changed, 23 insertions(+), 2 deletions(-)
>> 
>> 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];
>> 
>>    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);
>>    }
>> 
>> +    /* 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 declaration time:
>
>         char percent_files[8] = "";
>
> 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_count(fs->super));
>> +    } else {
>> +        snprintf(percent_blocks, sizeof(percent_blocks), " ");
>> +    }
>
> This could similarly be set at initialization:
>
>         char percent_blocks[8] = "";
>

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 = 100000;
>>    if (fs->super->s_max_mnt_count > 0) {
>>        next_check = fs->super->s_max_mnt_count - fs->super->s_mnt_count;
>> -- 
>> 2.40.0
>>