[GitHub] [incubator-nuttx-apps] pkarashchenko commented on a diff in pull request #1214: system/hostname: Add an option to read the hostname from a file

[GitBox <gi...@apache.org>]

pkarashchenko commented on code in PR #1214:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1214#discussion_r912855777


##########
system/hostname/hostname_main.c:
##########
@@ -42,17 +42,64 @@
 int main(int argc, FAR char *argv[])
 {
   int ret;
+  FILE *f;
+  int maxarg = 2;
   char hostname[HOST_NAME_MAX + 1];
 
   if (argc > 1)
     {
-      if (argc > 2)
+      if (strcmp(argv[1], "-F") == 0)
+        {
+          if (argc < 3)
+            {
+              fprintf(stderr, "ERROR: Option requires an argument -- 'F'\n");
+              return EXIT_FAILURE;
+            }
+
+          f = fopen(argv[2], "r");
+          if (f == NULL)
+            {
+              fprintf(stderr, "ERROR: Failed to open '%s': %s\n", argv[2],
+                      strerror(errno));
+              return EXIT_FAILURE;
+            }
+
+          if (fgets(hostname, sizeof(hostname), f) == NULL && (errno != 0))
+            {
+              fclose(f);

Review Comment:
   `fclose` may modify `errno`, `fprintf` may print the value different from one that met `(errno != 0)` to true in the `if`.



##########
system/hostname/hostname_main.c:
##########
@@ -42,17 +42,64 @@
 int main(int argc, FAR char *argv[])
 {
   int ret;
+  FILE *f;

Review Comment:
   ```suggestion
     FAR FILE *f;
   ```



##########
system/hostname/hostname_main.c:
##########
@@ -42,17 +42,64 @@
 int main(int argc, FAR char *argv[])
 {
   int ret;
+  FILE *f;
+  int maxarg = 2;
   char hostname[HOST_NAME_MAX + 1];
 
   if (argc > 1)
     {
-      if (argc > 2)
+      if (strcmp(argv[1], "-F") == 0)
+        {
+          if (argc < 3)
+            {
+              fprintf(stderr, "ERROR: Option requires an argument -- 'F'\n");
+              return EXIT_FAILURE;
+            }
+
+          f = fopen(argv[2], "r");
+          if (f == NULL)
+            {
+              fprintf(stderr, "ERROR: Failed to open '%s': %s\n", argv[2],
+                      strerror(errno));
+              return EXIT_FAILURE;
+            }
+
+          if (fgets(hostname, sizeof(hostname), f) == NULL && (errno != 0))
+            {
+              fclose(f);
+              fprintf(stderr, "ERROR: Failed to read '%s': %s\n", argv[2],
+                      strerror(errno));
+              return EXIT_FAILURE;
+            }
+
+          *strchrnul(hostname, '\n') = '\0';
+
+          fclose(f);
+          maxarg = 3;
+        }
+      else if (argv[1][0] == '-')
+        {
+          fprintf(stderr, "ERROR: Invalid option -- '%s'\n", argv[1]);
+          return EXIT_FAILURE;
+        }
+      else
+        {
+          strlcpy(hostname, argv[1], sizeof(hostname));
+        }
+
+      if (argc > maxarg)
         {
           fprintf(stderr, "ERROR: Too many arguments\n");
           return EXIT_FAILURE;
         }
 
-      ret = sethostname(argv[1], strlen(argv[1]));
+      if (strlen(hostname) == 0)

Review Comment:
   Optimization (optional):
   Maybe we can somehow avoid double call of `strlen(hostname)` in case of non-null strings, like
   `if (*hostname == '\0')`?
   



-- 
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: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org