diff mbox

qemu-pr-helper: Actually allow users to specify pidfile

Message ID 6f10cd53d361a395aa0e85a9311ec4e9a8fc11e5.1521868451.git.mprivozn@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michal Privoznik March 24, 2018, 5:14 a.m. UTC
Due to wrong specification of arguments to getopt_long() any
attempt to set pidfile resulted in:

1) the default to be leaked
2) the @pidfile variable to be set to NULL (because optarg is
NULL without this patch).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 scsi/qemu-pr-helper.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Eric Blake March 24, 2018, 11:44 a.m. UTC | #1
On 03/24/2018 12:14 AM, Michal Privoznik wrote:
> Due to wrong specification of arguments to getopt_long() any
> attempt to set pidfile resulted in:
> 
> 1) the default to be leaked
> 2) the @pidfile variable to be set to NULL (because optarg is
> NULL without this patch).

Broken in introduction with commit b855f8d, in 2.11.

> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

CC: qemu-stable@nongnu.org
Reviewed-by: Eric Blake <eblake@redhat.com>
Eric Blake March 24, 2018, 11:46 a.m. UTC | #2
On 03/24/2018 12:14 AM, Michal Privoznik wrote:
> Due to wrong specification of arguments to getopt_long() any
> attempt to set pidfile resulted in:
> 
> 1) the default to be leaked
> 2) the @pidfile variable to be set to NULL (because optarg is
> NULL without this patch).

Broken since the introduction in commit b855f8d, in 2.11 (documentation 
has always mentioned the argument, code has never supported it).

> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

CC: qemu-stable@nongnu.org
Reviewed-by: Eric Blake <eblake@redhat.com>
Paolo Bonzini March 26, 2018, 11:10 a.m. UTC | #3
On 24/03/2018 12:46, Eric Blake wrote:
> On 03/24/2018 12:14 AM, Michal Privoznik wrote:
>> Due to wrong specification of arguments to getopt_long() any
>> attempt to set pidfile resulted in:
>>
>> 1) the default to be leaked
>> 2) the @pidfile variable to be set to NULL (because optarg is
>> NULL without this patch).
> 
> Broken since the introduction in commit b855f8d, in 2.11 (documentation
> has always mentioned the argument, code has never supported it).

Queued, thanks.

Paolo

>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> 
> CC: qemu-stable@nongnu.org
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
diff mbox

Patch

diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index 3facbba170..21e1b8ea60 100644
--- a/scsi/qemu-pr-helper.c
+++ b/scsi/qemu-pr-helper.c
@@ -903,12 +903,12 @@  static int drop_privileges(void)
 
 int main(int argc, char **argv)
 {
-    const char *sopt = "hVk:fdT:u:g:vq";
+    const char *sopt = "hVk:f:dT:u:g:vq";
     struct option lopt[] = {
         { "help", no_argument, NULL, 'h' },
         { "version", no_argument, NULL, 'V' },
         { "socket", required_argument, NULL, 'k' },
-        { "pidfile", no_argument, NULL, 'f' },
+        { "pidfile", required_argument, NULL, 'f' },
         { "daemon", no_argument, NULL, 'd' },
         { "trace", required_argument, NULL, 'T' },
         { "user", required_argument, NULL, 'u' },
@@ -952,7 +952,8 @@  int main(int argc, char **argv)
             }
             break;
         case 'f':
-            pidfile = optarg;
+            g_free(pidfile);
+            pidfile = g_strdup(optarg);
             break;
 #ifdef CONFIG_LIBCAP
         case 'u': {