"find -follow -type l" doesn't quite work.

[Rob Landley]

On Monday 05 October 2009 22:16:48 Mike Frysinger wrote:
> On Tuesday 29 September 2009 20:41:41 Denys Vlasenko wrote:
> > On Tuesday 29 September 2009 23:18, Rob Landley wrote:
> > > On Tuesday 29 September 2009 04:01:12 Denys Vlasenko wrote:
> > > > On Fri, Sep 25, 2009 at 11:35 AM, Rob Landley <rob at landley.net> wrote:
> > > > > An easy way to find all dead symlinks under /path is:
> > > > >
> > > > >  find -follow -type l /path
> > > > >
> > > > > Using gnu find, that will produce a list of symlinks that, when
> > > > > you've followed them as far as they can, are still symlinks.  Using
> > > > > busybox, it finds the same set of files, but instead of listing
> > > > > them to stdout it gives error messages about them.  This behavior
> > > > > is less useful.
> > > >
> > > > Please try attached patch.
> > > >
> > > > --
> > > > vda
> > >
> > > It's full of unrelated changes that conflict with the last release
> > > version:
> > >
> > > Applying
> > > /home/landley/firmware/firmware/sources/patches/busybox-findl.patch
> > > patching file findutils/find.c
> >
> > Can you try to apply just this to 1.15.1?
> >
> > http://busybox.net/downloads/fixes-1.15.1/busybox-1.15.1-find.patch
> >
> > It is planned to go into 1.15.2
>
> the posted patch isnt entirely correct.  it didnt protect the zeroing of
> the new xdev vals when xdev support is turned off.  ive fixed the patch on
> the server, but it seems the 1.15 branch isnt being kept up to date with
> the posted patches, so you'll have to take care of this when git is finally
> updated.
>
> --- busybox-1.15.1/findutils/find.c
> +++ busybox-1.15.1/findutils/find.c
> @@ -109,8 +109,8 @@ struct globals {
>         struct G_sizecheck { \
>                 char G_sizecheck[sizeof(G) > COMMON_BUFSIZE ? -1 : 1]; \
>         }; \
> -       G.xdev_dev = NULL; \
> -       G.xdev_count = 0; \
> +       IF_FEATURE_FIND_XDEV(G.xdev_dev = NULL;) \
> +       IF_FEATURE_FIND_XDEV(G.xdev_count = 0;) \
>         G.actions = NULL; \
>         G.need_print = 1; \
>         G.recurse_flags = ACTION_RECURSE; \
> -mike

Good catch.

But on an architectural note, I'm not sure #ifdeffing out global union entries 
really makes sense considering that the union is A) always going to be the 
largest size any applet uses as far as the bss layout is concerned, B) going 
to dirty an entire page of physical memory anyway.  As far as the code is 
concerned, all that changes is bss offsets, so having unused (and 
uninitialized) globals shouldn't make the _binary_ any bigger, and if this app 
isn't the largest user (and your struct is less than page size anyway and 
reasonably aligned) wouldn't actually add runtime memory usage either.

Do you have any numbers of what it saves?

What I did in toybox is move feature-specific entries to the end of the struct, 
and then just left the variable definitions in all the time but if()ed out the 
users and let the dead code elimination yank 'em.  Less chance of config-
specific build breaks that way, since the IF() macros are just prettier ways of 
doing #ifdef.  (Also the above might have been a NOP cleanup if you set fields 
that are never read from.  Still a cleanup, but not a space savings.  Dunno if 
the compiler's smart enough to figure that out with a global, though, it might 
need one of those --whole-tree flags or something.)

I keep meaning to write up a thorough comparison of the toybox and busybox 
base infrastructure designs, but my todo list runneth over...

Rob
-- 
Latency is more important than throughput. It's that simple. - Linus Torvalds