Re: [Qemu-trivial] [PATCH 1/4] qemu-char: fix parameter check for qemu_chr_open_pipe

[zhanghailiang <zhang.zhanghailiang@huawei.com>]

On 2014/11/2 13:18, Michael Tokarev wrote:
> 01.11.2014 04:50, zhanghailiang wrote:
>> The filename parameter never to be NULL, because in qemu_chr_parse_pipe
>> it is return value of g_strdup(device), where device will not be
>> NULL.
>>
>> We should check its length.
>> After this patch, when run command:
>>   qemu-system-x86_64 -chardev pipe,id=pipe1,path=
>> It will report error:
>>   chardev: pipe: no filename given
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> ---
>>   qemu-char.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index bd0709b..42b1d8f 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -1084,7 +1084,7 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
>>       char filename_out[CHR_MAX_FILENAME_SIZE];
>>       const char *filename = opts->device;
>>
>> -    if (filename == NULL) {
>> +    if (filename == NULL || strlen(filename) == 0) {
>>           fprintf(stderr, "chardev: pipe: no filename given\n");
>>           return NULL;
>>       }
>
> and chr_parse_pipe() looks like:
>
>
> static void qemu_chr_parse_pipe(QemuOpts *opts, ChardevBackend *backend,
>                                  Error **errp)
> {
>      const char *device = qemu_opt_get(opts, "path");
>
>      if (device == NULL) {
>          error_setg(errp, "chardev: pipe: no device path given");
>          return;
>      }
>
> Maybe we should combine the two checks into one in chr_parse_pipe,
> and remove the check in chr_open_pipe entirely?

Good idea, This is just like what qemu_chr_parse_udp does ;) Thanks.

> .
>