edwardcapriolo commented on code in PR #8184:
URL: https://github.com/apache/hadoop/pull/8184#discussion_r2721296035


##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c:
##########
@@ -1370,34 +1389,74 @@ int create_container_log_dirs(const char *container_id, 
const char *app_id,
   return 0;
 }
 
-/**
- * Function to create the application directories.
- * Returns pointer to primary_app_dir or NULL if it fails.
- */
-static char *create_app_dirs(const char *user,
+char* concat(const char *s1, const char *s2) {
+    size_t len1 = strlen(s1);
+    size_t len2 = strlen(s2);
+    char *result = malloc(len1 + len2 + 1);
+    if (result == NULL) {
+        exit(EXIT_FAILURE);
+    }
+    memcpy(result, s1, len1);
+    memcpy(result + len1, s2, len2 + 1);
+    return result;
+}
+
+void maybe_create_appcache(const char *appcache, mode_t permissions){
+  //fprintf(LOGFILE, "Going to create %s\n", appcache);
+  struct stat exists;
+  int stat_res = stat(appcache, &exists);
+  if (stat_res == -1){
+    int mk_res = mkdir(appcache, permissions);
+    fprintf(LOGFILE, "Creating appcache %s result %d\n", appcache, mk_res);
+  }
+}
+
+char *create_app_dirs(const char *user,
                              const char *app_id,
-                             char* const* local_dirs)
-{
+                             char* const* local_dirs) {
   // 750
   mode_t permissions = S_IRWXU | S_IRGRP | S_IXGRP;
   char* const* nm_root;
   char *primary_app_dir = NULL;
-  for(nm_root=local_dirs; *nm_root != NULL; ++nm_root) {
+  for(nm_root = local_dirs; *nm_root != NULL; ++nm_root) {
     char *app_dir = get_app_directory(*nm_root, user, app_id);
-    if (app_dir == NULL) {
-      // try the next one
-    } else if (strstr(app_dir, "..") != 0) {
-      fprintf(LOGFILE, "Unsupported app directory path detected.\n");
+    fprintf(LOGFILE, "Appdir %s\n", app_dir);
+    if (app_dir == NULL){
       free(app_dir);
-    } else if (mkdirs(app_dir, permissions) != 0) {
+      continue;
+    }
+    //implementation node: Could be more thought put in do this detection
+    if (strstr(app_dir, "..") != 0) {
+      fprintf(LOGFILE, "Unsupported app directory path detected.\n");
       free(app_dir);
-    } else if (primary_app_dir == NULL) {
-      primary_app_dir = app_dir;

Review Comment:
   This was a leak in the code as the pointer for primary_add_dir points to 
app_dir which we later free. This leads to undefined behavior.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to