[
https://issues.apache.org/jira/browse/HADOOP-4491?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12731883#action_12731883
]
Hemanth Yamijala commented on HADOOP-4491:
------------------------------------------
I looked at the C code that was pending from previous review. Looking good
overall. I have a few comments - mostly minor:
- Should the pointer returned by strdup in configuration.get_value() be freed ?
It doesn't seem to be in some cases.
- Return value of any code path calling concatenate should be checked, as it
can be null.
- I think we can use stat instead of opendir to check for existence of path.
- Check return value of fts calls.
- I think it may be good to use the flag FTS_NOSTAT as we don't use stat info
and it looks like it is an optimization to do so.
- It may be safe to handle a default case in secure_path, just in case versions
of FTS have types we are not handling. For e.g we are not handling FTS_ERR.
- The value of the process_flag seems against the name. It should be true (non
zero) if we want to process and false (zero) otherwise. Then code like if
(!process_path) continue; can be used. Similarly for the variables 'failed' and
'first_task'
- We can use strerror that will print a standard message per errno itself, for
calls like chown etc. rather than printing the error message ourselves based on
errno.
- What are the permissions for the tasklog attempt directories ? Since they're
not being set, will they take the value of the default umask ?
- check_owner is removed. Need to make sure this is ok.
- Let's add debug prints into #ifdef DEBUG or something similar.
- Looks like run_task_as_user shares code with initialize_task. Can it call
initialize_task directly ?
- In TERMINATE_TASK_JVM, we are not using first_task - remove the argument to
task-controller. Also for clarity, I think we can call the argument as
cleanup_work_dirs or something like that, rather than first_task.
- There seems to be a spurious call to open_log_file in TERMINATE_TASK_JVM.
- display_usage seems out of place in task-controller.c. Can we move it to main
?
> Per-job local data on the TaskTracker node should have right access-control
> ---------------------------------------------------------------------------
>
> Key: HADOOP-4491
> URL: https://issues.apache.org/jira/browse/HADOOP-4491
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: security
> Reporter: Arun C Murthy
> Assignee: Vinod K V
> Attachments: HADOOP-4491-20090623-common.1.txt,
> HADOOP-4491-20090623-mapred.1.txt, HADOOP-4491-20090703-common.1.txt,
> HADOOP-4491-20090703-common.txt, HADOOP-4491-20090703.1.txt,
> HADOOP-4491-20090703.txt, HADOOP-4491-20090707-common.txt,
> HADOOP-4491-20090707.txt, HADOOP-4491-20090716-mapred.txt
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
