libatalk/conf: code cleanup and add locking to get_vol_uuid()

Remove whitespace, always use {}, split function calls and if checks.

Only open the UUID file once and add lock protection.

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: HAT <hat@fa2.so-net.ne.jp>

libatalk/util/netatalk_conf.c

@@ -79,78 +79,116 @@ static char *get_vol_uuid(const AFPObj *obj, const char *volname)
     char *volname_conf;
     char buf[1024], uuid[UUID_PRINTABLE_STRING_LENGTH], *p;
     FILE *fp;
-    struct stat tmpstat;
-    int fd;
-
-    if ((fp = fopen(obj->options.uuidconf, "r")) != NULL) {  /* read open? */
+    int result;
+    long offset;
+
+    fp = fopen(obj->options.uuidconf, "r+");
+    if (fp != NULL) {
+        /* Lock the file */
+        result = flock(fileno(fp), LOCK_SH);
+        if (result != 0) {
+            LOG(log_error, logtype_afpd, "Can't lock UUID file %s", obj->options.uuidconf);
+            return NULL;
+        }
+
         /* scan in the conf file */
-        while (fgets(buf, sizeof(buf), fp) != NULL) { 
+        while (fgets(buf, sizeof(buf), fp) != NULL) {
             p = buf;
-            while (p && isblank(*p))
+            while (p && isblank(*p)) {
                 p++;
-            if (!p || (*p == '#') || (*p == '\n'))
-                continue;                             /* invalid line */
+            }
+
+            if (!p || (*p == '#') || (*p == '\n')) {
+                /* invalid line */
+                continue;
+            }
+
             if (*p == '"') {
                 p++;
-                if ((volname_conf = strtok( p, "\"" )) == NULL)
-                    continue;                         /* syntax error */
+                volname_conf = strtok( p, "\"" );
+                if (volname_conf == NULL) {
+                    /* syntax error */
+                    continue;
+                }
             } else {
-                if ((volname_conf = strtok( p, " \t" )) == NULL)
-                    continue;                         /* syntax error: invalid name */
+                volname_conf = strtok( p, " \t" );
+                if (volname_conf == NULL) {
+                    /* syntax error: invalid name */
+                    continue;
+                }
             }
             p = strchr(p, '\0');
             p++;
-            if (*p == '\0')
-                continue;                             /* syntax error */
-
-            if (strcmp(volname, volname_conf) != 0)
-                continue;                             /* another volume name */
-
-            while (p && isblank(*p))
+            if (*p == '\0') {
+                /* syntax error */
+                continue;
+            }
+
+            if (strcmp(volname, volname_conf) != 0) {
+                /* another volume name */
+                continue;
+            }
+
+            while (p && isblank(*p)) {
                 p++;
+            }
 
             if (sscanf(p, "%36s", uuid) == 1 ) {
-                for (int i=0; uuid[i]; i++)
+                for (int i=0; uuid[i]; i++) {
                     uuid[i] = toupper(uuid[i]);
+                }
                 LOG(log_debug, logtype_afpd, "get_uuid('%s'): UUID: '%s'", volname, uuid);
                 fclose(fp);
                 return strdup(uuid);
             }
         }
     }
 
-    if (fp)
-        fclose(fp);
-
-    /*  not found or no file, reopen in append mode */
-
-    if (stat(obj->options.uuidconf, &tmpstat)) {                /* no file */
-        if (( fd = creat(obj->options.uuidconf, 0644 )) < 0 ) {
-            LOG(log_error, logtype_afpd, "ERROR: Cannot create %s (%s).",
+    if (fp == NULL) {
+        fp = fopen(obj->options.uuidconf, "a");
+        if (fp == NULL) {
+            LOG(log_error, logtype_afpd, "Cannot create or append to %s (%s)",
                 obj->options.uuidconf, strerror(errno));
             return NULL;
         }
-        if (( fp = fdopen( fd, "w" )) == NULL ) {
-            LOG(log_error, logtype_afpd, "ERROR: Cannot fdopen %s (%s).",
-                obj->options.uuidconf, strerror(errno));
-            close(fd);
-            return NULL;
-        }
-    } else if ((fp = fopen(obj->options.uuidconf, "a+")) == NULL) { /* not found */
-        LOG(log_error, logtype_afpd, "Cannot create or append to %s (%s).",
+    }
+
+    result = flock(fileno(fp), LOCK_EX);
+    if (result != 0) {
+        LOG(log_error, logtype_afpd, "Can't lock UUID file %s (%s)",
             obj->options.uuidconf, strerror(errno));
+        fclose(fp);
+        return NULL;
+    }
+
+    result = fseek(fp, 0L, SEEK_END);
+    if (result != 0) {
+        LOG(log_error, logtype_afpd, "Can't fseek to end of UUID file %s (%s)",
+            obj->options.uuidconf, strerror(errno));
+        fclose(fp);
         return NULL;
     }
-    fseek(fp, 0L, SEEK_END);
-    if(ftell(fp) == 0) {                     /* size = 0 */
+
+    offset = ftell(fp);
+    if (offset == 0) {
         fprintf(fp, "# DON'T TOUCH NOR COPY THOUGHTLESSLY!\n");
         fprintf(fp, "# This file is auto-generated by afpd\n");
         fprintf(fp, "# and stores UUIDs for Time Machine volumes.\n\n");
     } else {
-        fseek(fp, -1L, SEEK_END);
-        if(fgetc(fp) != '\n') fputc('\n', fp); /* last char is \n? */
-    }                    
-
+        result = fseek(fp, -1L, SEEK_END);
+        if (result != 0) {
+            LOG(log_error, logtype_afpd, "Can't fseek SEEK_END-1 UUID file %s (%s)",
+                obj->options.uuidconf, strerror(errno));
+            fclose(fp);
+            return NULL;
+        }
+
+        if(fgetc(fp) != '\n') {
+            /* last char is \n? */
+            fputc('\n', fp);
+        }
+    }
+
     /* generate uuid and write to file */
     atalk_uuid_t id;
     const char *cp;
@@ -161,7 +199,7 @@ static char *get_vol_uuid(const AFPObj *obj, const char *volname)
 
     fprintf(fp, "\"%s\"\t%36s\n", volname, cp);
     fclose(fp);
-    
+
     return strdup(cp);
 }