zhanghailiang <zhang.zhanghaili...@huawei.com> writes:
> For functions qemu_chr_parse_spice_vmc and qemu_chr_parse_spice_port,
> we should also check if parameter name is empty, and it will help finding
> the wrong configure, such as
> 'qemu-system-x86_64 -chardev spiceport(or spiceport),id=id,name='
>
> After check the parameter in parse function, we can remove the
> superfluous parameter checks in qemu_chr_open_spice_*
>
> Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com>
> ---
> spice-qemu-char.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/spice-qemu-char.c b/spice-qemu-char.c
> index 8106e06..d3c1f5c 100644
> --- a/spice-qemu-char.c
> +++ b/spice-qemu-char.c
> @@ -290,11 +290,6 @@ CharDriverState *qemu_chr_open_spice_vmc(const char
> *type)
> {
> const char **psubtype = spice_server_char_device_recognized_subtypes();
>
> - if (type == NULL) {
> - fprintf(stderr, "spice-qemu-char: missing name parameter\n");
> - print_allowed_subtypes();
> - return NULL;
> - }
As with patch 1 I'd at least assert the contract. But this also drops a
helpful info dump on a missing parameter.
> for (; *psubtype != NULL; ++psubtype) {
> if (strcmp(type, *psubtype) == 0) {
> break;
> @@ -315,11 +310,6 @@ CharDriverState *qemu_chr_open_spice_port(const char
> *name)
> CharDriverState *chr;
> SpiceCharDriver *s;
>
> - if (name == NULL) {
> - fprintf(stderr, "spice-qemu-char: missing name parameter\n");
> - return NULL;
> - }
> -
> chr = chr_open("port", spice_port_set_fe_open);
> s = chr->opaque;
> s->sin.portname = g_strdup(name);
> @@ -345,7 +335,7 @@ static void qemu_chr_parse_spice_vmc(QemuOpts *opts,
> ChardevBackend *backend,
> {
> const char *name = qemu_opt_get(opts, "name");
>
> - if (name == NULL) {
> + if (name == NULL || !name[0]) {
> error_setg(errp, "chardev: spice channel: no name given");
> return;
> }
> @@ -358,7 +348,7 @@ static void qemu_chr_parse_spice_port(QemuOpts *opts,
> ChardevBackend *backend,
> {
> const char *name = qemu_opt_get(opts, "name");
>
> - if (name == NULL) {
> + if (name == NULL || !name[0]) {
> error_setg(errp, "chardev: spice port: no name given");
> return;
> }
Same comments as for 1/5
--
Alex Bennée
