You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/07/03 21:22:15 UTC
[GitHub] [incubator-nuttx-apps] normanr opened a new pull request, #1214: system/hostname: Add an option to read the hostname from a file
normanr opened a new pull request, #1214:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1214
## Summary
Add an option to read the hostname from a file (eg: in `/etc/init.d/rcS`). Add a check that the hostname is non-empty.
## Impact
Makes it possible to set hostname from a file without `NSH_CMDPARMS` enabled (eg: "hostname \`cat /etc/hostname\`"). Ensures that hostname must be non-blank.
## Testing
```
nsh> hostname -F /data/hostname
nsh> hostname -F /tmp/missing
ERROR: Failed to open '/tmp/missing': No such file or directory
nsh> hostname ""
ERROR: The specified hostname is invalid
nsh> echo > /tmp/newline
nsh> hostname -F /tmp/newline
ERROR: The specified hostname is invalid
nsh> echo -n > /tmp/empty
nsh> hostname -F /tmp/empty
ERROR: The specified hostname is invalid
nsh>
```
--
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
[GitHub] [incubator-nuttx-apps] normanr commented on a diff in pull request #1214: system/hostname: Add an option to read the hostname from a file
Posted by GitBox <gi...@apache.org>.
normanr commented on code in PR #1214:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1214#discussion_r922898636
##########
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)
Review Comment:
Done
--
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
[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
Posted by 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
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1214: system/hostname: Add an option to read the hostname from a file
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1214:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1214#discussion_r912592476
##########
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)
Review Comment:
should we add an usage?
--
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
[GitHub] [incubator-nuttx-apps] normanr commented on a diff in pull request #1214: system/hostname: Add an option to read the hostname from a file
Posted by GitBox <gi...@apache.org>.
normanr commented on code in PR #1214:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1214#discussion_r913272734
##########
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:
I considered this, and the resulting assembly code is identical. The compiler optimizes `strlen(hostname) == 0` as `*hostname == '\0'` so I figured to stick with the more readable version.
I'm not sure if we also want a check for `strlen(hostname) > HOST_NAME_MAX` (to prevent the supplied hostname from being silently truncated). I would expect that the compiler should re-use the returned value and not call `strlen(hostname)` twice.
--
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
[GitHub] [incubator-nuttx-apps] normanr commented on a diff in pull request #1214: system/hostname: Add an option to read the hostname from a file
Posted by GitBox <gi...@apache.org>.
normanr commented on code in PR #1214:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1214#discussion_r913272734
##########
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:
I considered this, and the resulting assembly code is identical. The compiler optimizes `strlen(hostname) == 0` as `*hostname == '\0'` so I figured to stick with the more readable version.
I'm not sure if we also want a check for `strlen(hostname) > HOST_NAME_MAX` (to prevent the supplied hostname being truncated. I would expect that the compiler should re-use the returned value and not call `strlen(hostname)` twice.
--
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
[GitHub] [incubator-nuttx-apps] normanr commented on a diff in pull request #1214: system/hostname: Add an option to read the hostname from a file
Posted by GitBox <gi...@apache.org>.
normanr commented on code in PR #1214:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1214#discussion_r913272734
##########
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:
I considered this, and the resulting assembly code is identical. The compiler optimizes `strlen(hostname) == 0` as `*hostname == '\0'` so I figured to stick with the more readable version.
I'm not sure if we also want a check for `strlen(hostname) > HOST_NAME_MAX` (to prevent the supplied hostname from being silently truncated. I would expect that the compiler should re-use the returned value and not call `strlen(hostname)` twice.
--
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
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 merged pull request #1214: system/hostname: Add an option to read the hostname from a file
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #1214:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1214
--
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