Re: [PATCH] refactoring: users: Remove useless `trimmed_name` variable

[Kaz Kylheku]

On 2022-11-21 14:37, Alireza Arzehgar wrote:
> Dear All,
> 
> When I was reading users.c source code,  on the `list_entries_users`
> function, I saw that some codes can be removed and we can refactor this
> codes to simpler and better code.
> 
> I think this code is useless:
> 
>        if (IS_USER_PROCESS (this))
>         {
>           char *trimmed_name;
> 
>           trimmed_name = extract_trimmed_name (this);
> 
>           u[n_entries] = trimmed_name;
>           ++n_entries;
>         }
> 
> We can remove 5 lines of code and just put following code instead of
> removed codes:
>         if (IS_USER_PROCESS (this))
>             u[n_entries++] = extract_trimmed_name (this);

I agree with this (assuming that extract_trimmed_name
has nothing to do with the n_entries variable).

I agree with this because the local variable's name doesn't add
any documentary value. If the code were this:

        if (IS_USER_PROCESS (this))
         {
           char *trimmed_name;
 
           trimmed_name = xtrnam (this); // what does this do?
 
           u[n_entries] = trimmed_name;
           ++n_entries;
         }

then I would only suggest combining the declaration and assignment, because
it's not obvious that xtrnam produces the trimmed name of the object.
The intermediate variable helps to document what the cryptically-named
function is returning. A higher priority code improvement would
be renaming the function:

        if (IS_USER_PROCESS (this))
         {
           char *trimmed_name = xtrnam (this);
           u[n_entries] = trimmed_name;
           ++n_entries;
         }
> 
> I think replacing previous code can have the following benefits: (a) Less
> bulky code. (b) Less binary size.

(b) is almost certainly false, unless the program is built without
optimizations (-O0)

> For less binary size I examined and analyzed binaries before and after
> refactoring.
> 
> $ wc original-users refactored-users
>    951   4295 164400 original-users
>    949   4278 164328 refactored-users
> 
> In this case binary size was reduced. But the following test shows me that
> the text portion size will increase on the refactored version.
> 
> $ size original-users refactored-users
>    text      data    bss     dec      hex       filename
>   29495   1272    440  31207   79e7 original-users
>   29527   1272    440  31239   7a07 refactored-users

If your only change was eliminating a temporary, block-scoped variable
that is only used once, and you're seeing not only a significant
difference in the code size, but in the size of zero-initialized data,
something is suspicious.

Also note that the raw size of the file you are seeing is
vastly larger than what is reported by size for the code and data,
which means there is a lot of debug symbol material in there.

Did you do something like this:

  1. build it with your change: and record sizes
  2. Make sure "git diff" shows only the one change you're investigating.
  3. "git stash save" to stow away your changes; build and record sizes