Marc-André Lureau <marcandre.lur...@gmail.com> writes: > Hi > > On Mon, Mar 7, 2016 at 8:25 PM, Markus Armbruster <arm...@redhat.com> wrote: >> Option -m NAME is interpreted as directory name if we can statfs() it >> and its on hugetlbfs. Else it's interpreted as POSIX shared memory >> object name. This is nuts. >> >> Always interpret -m as directory. Create new -M for POSIX shared >> memory. Last of -m or -M wins. >> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- > > I don't see why the last should win is a good idea, see attached patch
Last one wins is pretty common behavior. In fact, it's what this program does for every single option with an argument. I didn't feel like making -m and -M special. > for a possible solution, also changing a few comments. Feel free to > squash it in this patch or include it in your series. I got a few comments inline. > Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com> Thanks! [...] > From e8112678496fd873ceaa34b3169e516130075ed4 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= <marcandre.lur...@redhat.com> > Date: Tue, 8 Mar 2016 20:31:09 +0100 > Subject: [PATCH] ivshmem-server: expect either -m or -M > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> > --- > contrib/ivshmem-server/main.c | 21 ++++++++++++--------- > 1 file changed, 12 insertions(+), 9 deletions(-) > > diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/main.c > index 2795db5..368fc67 100644 > --- a/contrib/ivshmem-server/main.c > +++ b/contrib/ivshmem-server/main.c > @@ -77,7 +77,7 @@ ivshmem_server_parse_args(IvshmemServerArgs *args, int > argc, char *argv[]) > "F" /* foreground */ > "p:" /* pid_file */ > "S:" /* unix_socket_path */ > - "m:" /* shm_path */ > + "m:" /* dirname */ The existing comments all name the member of args set by the option. There is no member dirname. > "M:" /* shm_path */ > "l:" /* shm_size */ > "n:" /* n_vectors */ > @@ -106,13 +106,15 @@ ivshmem_server_parse_args(IvshmemServerArgs *args, int > argc, char *argv[]) > break; > > case 'M': /* shm_path */ > - args->shm_path = optarg; > - args->use_shm_open = true; > - break; > + case 'm': /* dirname */ > + if (args->shm_path) { > + fprintf(stderr, "Please specify either -m or -M.\n"); > + ivshmem_server_help(argv[0]); > + exit(1); > + } > > - case 'm': /* shm_path */ > args->shm_path = optarg; > - args->use_shm_open = false; > + args->use_shm_open = c == 'M'; I think I'll steal this idea :) > break; > > case 'l': /* shm_size */ > @@ -207,7 +209,7 @@ main(int argc, char *argv[]) > .foreground = IVSHMEM_SERVER_DEFAULT_FOREGROUND, > .pid_file = IVSHMEM_SERVER_DEFAULT_PID_FILE, > .unix_socket_path = IVSHMEM_SERVER_DEFAULT_UNIX_SOCK_PATH, > - .shm_path = IVSHMEM_SERVER_DEFAULT_SHM_PATH, > + .shm_path = NULL, > .use_shm_open = true, > .shm_size = IVSHMEM_SERVER_DEFAULT_SHM_SIZE, > .n_vectors = IVSHMEM_SERVER_DEFAULT_N_VECTORS, > @@ -237,8 +239,9 @@ main(int argc, char *argv[]) > > /* init the ivshms structure */ > if (ivshmem_server_init(&server, args.unix_socket_path, > - args.shm_path, args.use_shm_open, > - args.shm_size, args.n_vectors, args.verbose) < > 0) { > + args.shm_path ?: IVSHMEM_SERVER_DEFAULT_SHM_PATH, > + args.use_shm_open, args.shm_size, args.n_vectors, > + args.verbose) < 0) { > fprintf(stderr, "cannot init server\n"); > goto err; > }