Re: [PATCH] ifplugd: simple code shrink (~ -43)

[Maksym Kryzhanovskyy]

I does the following:

$ git clone git://busybox.net/busybox.git
$ cd busybox
$ make defconfig
$ make
$ make baseline
$ patch -p1 < ../busybox.ifplugd.logic.patch
$ make
$ make bloatcheck

It still counts -76 bytes. What is wrong?

The set_ifreq_to_ifname I changed because of the WLAN function
uses sizeof(struct iwreq), but it is larger...


> -static void set_ifreq_to_ifname(struct ifreq *ifreq)
> +static void set_ifreq_to_ifname(void *req, size_t sz)
>  {
> -       memset(ifreq, 0, sizeof(struct ifreq));
> -       strncpy_IFNAMSIZ(ifreq->ifr_name, G.iface);
> +       struct req {
> +               union {
> +                       char ifname[IFNAMSIZ];
> +               } u;
> +       };
> +       memset(req, 0, sz);
> +       strncpy_IFNAMSIZ(((struct req *)req)->u.ifname, G.iface);
>  }
> 
> What's the point of adding the sz parameter, if it is always
> sizeof(struct ifreq) in every callsite?
> 
> 
> > function                                             old     new   delta
> > ifplugd_main                                        1056    1119     +63
> > maybe_up_new_iface                                    33      35      +2
> > detect_link                                          216     208      -8
> > detect_link_wlan                                     120      99     -21
> > set_ifreq_to_ifname                                   32       -     -32
> > static.method                                         40       -     -40
> > .rodata                                           133272  133232     -40
> >
> ------------------------------------------------------------------------------
> > (add/remove: 0/2 grow/shrink: 2/3 up/down: 65/-141)           Total: -76
> bytes
> 
> I see this instead:
> 
> function                                             old     new   delta
> ifplugd_main                                        1055    1130     +75
> up_iface                                             112     117      +5
> detect_link_priv                                      80      85      +5
> detect_link_mii                                       80      85      +5
> detect_link_iff                                       71      76      +5
> detect_link_ethtool                                   65      70      +5
> maybe_up_new_iface                                    33      36      +3
> set_ifreq_to_ifname                                   36      33      -3
> detect_link                                          217     214      -3
> detect_link_wlan                                     125     105     -20
> static.method                                         40       -     -40
> ------------------------------------------------------------------------------
> (add/remove: 0/1 grow/shrink: 7/3 up/down: 103/-66)            Total: 37 bytes
> 
> and with set_ifreq_to_ifname change rolled back, it is still
> getting bigger, not smaller:
> 
> ifplugd_main                                        1055    1125     +70
> maybe_up_new_iface                                    33      36      +3
> detect_link                                          217     214      -3
> static.method                                         40       -     -40
> ------------------------------------------------------------------------------
> (add/remove: 0/1 grow/shrink: 2/1 up/down: 73/-43)             Total: 30 bytes
>    text	   data	    bss	    dec	    hex	filename
>  837992	   7528	      0	 845520	  ce6d0	busybox_old
>  838022	   7528	      0	 845550	  ce6ee	busybox_unstripped
> 
> 
> > The second patch includes rename the named constants acording to bb
> convention
> > and removes those which are not used. No size changes.
> 
> The second patch should include only new changes, not all changes combined.
> Otherwise it's harder to review.
> 
> -- 
> vda
> 
> 
>