[Toybox] Regarding if() condition in dirtree_add_node

[Rob Landley]

On 05/21/2013 12:36:41 AM, Sandeep Sharma wrote:
> Hi Rob,
> 
>                I have came across a condition in dirtree_add_node(),  
> where
> we are reading the link even though symfollow is not set :-
> 
>                if (S_ISLNK(st.st_mode) {
>                if (0>(linklen = readlinkat(fd, name, buf, 4095))) goto
> error;
>                buf[linklen++]=0;
>                }
>               In this senario if we land in readonly links it shouts  
> that
> the link is not present, even thogh we are not following links.
>               I think condition should be
>               if (S_ISLNK(st.st_mode) && symfollow). whats your  
> opinion?

It's not trying to open the file the link points to, it's trying to  
read the link's value. The symfollow thing is to treate symlinks as  
directories and recurse through them, this is just trying to fill out  
the current node's data, and that data should include the value of the  
symlink if this is a symlink.

Could you give me an example that would let me reproduce the failure  
you're seeing?

By the way, I've had this in my tree for months:

--- a/lib/dirtree.c     Tue May 21 00:23:23 2013 -0500
+++ b/lib/dirtree.c     Tue May 21 05:13:54 2013 -0500
@@ -138,7 +138,7 @@
    struct dirent *entry;
    DIR *dir;

-  if (!(dir = fdopendir(node->data))) {
+  if (node->data == -1 || !(dir = fdopendir(node->data))) {
      char *path = dirtree_path(node, 0);
      perror_msg("No %s", path);
      free(path);

It's fallout from running the full test suite twice (make defconfig;  
make tests; make tests), which produces entires with crazy permissions  
(such as chmod 000 dirname) that don't get cleaned up by the previous  
run, and the result makes the ls command misbehave.

   ls: No one: Bad file descriptor

The crazy permissions are intentional, part of the testing. The lack of  
cleanup is one problem. The bugs it's revealed in ls are another. I've  
been meaning to do the follow-up work on this to track it down and fix  
it, but I just haven't had time, and new todo items keep coming in  
faster than I can keep up with them. (Sorry, I feel bad about this, but  
I'm not as good at operating on 4 hours of sleep as I used to be.)

My current contract is up at the end of July and as always I get a  
little time off between gigs, but last time I didn't remotely catch up  
with the backlog before heading off to the new job. So I'm putting  
together an kickstarter-like campaign (indiegogo actually) to see if  
any of the companies interested in toybox want to anonymously throw  
money at me to work on it full time for a bit instead of doing  
something else.

If not, I'll keep working on it as I've been doing and at least no  
longer feel guilty about not keeping up. :)

>              And one more thing:-
>              I am using xreadfile() function present in lib.c but it
> is commented. Do you mind uncommenting it?

Sure, but I prefer to do so as part of checking in code that uses it.  
(The reason it's commented right now is it seems useful, but nothing in  
the tree is using it yet.)

Rob