[ 
https://issues.apache.org/jira/browse/YARN-4594?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15127260#comment-15127260
 ] 

Colin Patrick McCabe commented on YARN-4594:
--------------------------------------------

Thanks for the careful review, [~jlowe].

bq. The unit test fails, probably because we now deference NULL instead of 
terminating the loop in this code. When readdir returns NULL and there's no 
error, the code will try to dereference a null de.

Ah, sorry.  There is a missing {{break}}, let me fix that.

bq. The code can end up returning success despite an allocation failure because 
it doesn't initialize ret in this error case:

Good catch.  {{ret}} is initialized at the top of the function to prevent 
undefined behavior, but it should be changed to {{ENOMEM}} there to reflect the 
memory allocation failure.

bq. This code would be simpler to read if it didn't negate the error when 
trying to deal with it:

changed

> container-executor fails to remove directory tree when chmod required
> ---------------------------------------------------------------------
>
>                 Key: YARN-4594
>                 URL: https://issues.apache.org/jira/browse/YARN-4594
>             Project: Hadoop YARN
>          Issue Type: Bug
>          Components: nodemanager
>            Reporter: Colin Patrick McCabe
>            Assignee: Colin Patrick McCabe
>         Attachments: YARN-4594.001.patch, YARN-4594.002.patch, 
> YARN-4594.003.patch
>
>
> test-container-executor.c doesn't work:
> * It assumes that realpath(/bin/ls) will be /bin/ls, whereas it is actually 
> /usr/bin/ls on many systems.
> * The recursive delete logic in container-executor.c fails -- nftw does the 
> wrong thing when confronted with directories with the wrong mode (permission 
> bits), leading to an attempt to run rmdir on a non-empty directory.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to