| Message ID | 20210709160843.339796-2-li.zhang@ionos.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,RESEND,1/2] qmp: Support chardev-change | expand |
Ping On Fri, Jul 9, 2021 at 6:11 PM Li Zhang <zhlcindy@gmail.com> wrote: > > To improve the problematic source code in qmp_chardev_change > and some redundant source code, the functions qmp_chardev_add > and qmp_chardev_change are refactored. It is mentioned in thread: > https://lists.nongnu.org/archive/html/qemu-devel/2021-04/msg03167.html > > Signed-off-by: Li Zhang <li.zhang@ionos.com> > --- > chardev/char.c | 72 ++++++++++++++++++++++++++------------------------ > 1 file changed, 37 insertions(+), 35 deletions(-) > > diff --git a/chardev/char.c b/chardev/char.c > index d959eec522..cb6f287cd2 100644 > --- a/chardev/char.c > +++ b/chardev/char.c > @@ -1028,23 +1028,10 @@ Chardev *qemu_chardev_new(const char *id, const char *typename, > return chr; > } > > -ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend, > - Error **errp) > +static ChardevReturn *chardev_add(const char *id, Chardev *chr, > + Error **errp) > { > - const ChardevClass *cc; > ChardevReturn *ret; > - Chardev *chr; > - > - cc = char_get_class(ChardevBackendKind_str(backend->type), errp); > - if (!cc) { > - return NULL; > - } > - > - chr = chardev_new(id, object_class_get_name(OBJECT_CLASS(cc)), > - backend, NULL, false, errp); > - if (!chr) { > - return NULL; > - } > > if (!object_property_try_add_child(get_chardevs_root(), id, OBJECT(chr), > errp)) { > @@ -1062,6 +1049,26 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend, > return ret; > } > > +ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend, > + Error **errp) > +{ > + const ChardevClass *cc; > + Chardev *chr; > + > + cc = char_get_class(ChardevBackendKind_str(backend->type), errp); > + if (!cc) { > + return NULL; > + } > + > + chr = chardev_new(id, object_class_get_name(OBJECT_CLASS(cc)), > + backend, NULL, false, errp); > + if (!chr) { > + return NULL; > + } > + > + return chardev_add(id, chr, errp); > +} > + > ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, > Error **errp) > { > @@ -1070,7 +1077,6 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, > Chardev *chr, *chr_new; > bool closed_sent = false; > bool handover_yank_instance; > - ChardevReturn *ret; > > chr = qemu_chr_find(id); > if (!chr) { > @@ -1089,11 +1095,22 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, > return NULL; > } > > + cc = CHARDEV_GET_CLASS(chr); > + cc_new = char_get_class(ChardevBackendKind_str(backend->type), errp); > + if (!cc_new) { > + return NULL; > + } > + > be = chr->be; > if (!be) { > /* easy case */ > - object_unparent(OBJECT(chr)); > - return qmp_chardev_add(id, backend, errp); > + chr_new = chardev_new(id, object_class_get_name(OBJECT_CLASS(cc_new)), > + backend, NULL, false, errp); > + if (!chr_new) { > + return NULL; > + } > + > + goto out; > } > > if (!be->chr_be_change) { > @@ -1101,12 +1118,6 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, > return NULL; > } > > - cc = CHARDEV_GET_CLASS(chr); > - cc_new = char_get_class(ChardevBackendKind_str(backend->type), errp); > - if (!cc_new) { > - return NULL; > - } > - > /* > * The new chardev should not register a yank instance if the current > * chardev has registered one already. > @@ -1147,18 +1158,9 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, > */ > chr->handover_yank_instance = handover_yank_instance; > > +out: > object_unparent(OBJECT(chr)); > - object_property_add_child(get_chardevs_root(), chr_new->label, > - OBJECT(chr_new)); > - object_unref(OBJECT(chr_new)); > - > - ret = g_new0(ChardevReturn, 1); > - if (CHARDEV_IS_PTY(chr_new)) { > - ret->pty = g_strdup(chr_new->filename + 4); > - ret->has_pty = true; > - } > - > - return ret; > + return chardev_add(id, chr_new, errp); > } > > void qmp_chardev_remove(const char *id, Error **errp) > -- > 2.25.1 >
Hi all, Any comments for this patch? On Fri, Jul 9, 2021 at 6:11 PM Li Zhang <zhlcindy@gmail.com> wrote: > > To improve the problematic source code in qmp_chardev_change > and some redundant source code, the functions qmp_chardev_add > and qmp_chardev_change are refactored. It is mentioned in thread: > https://lists.nongnu.org/archive/html/qemu-devel/2021-04/msg03167.html > > Signed-off-by: Li Zhang <li.zhang@ionos.com> > --- > chardev/char.c | 72 ++++++++++++++++++++++++++------------------------ > 1 file changed, 37 insertions(+), 35 deletions(-) > > diff --git a/chardev/char.c b/chardev/char.c > index d959eec522..cb6f287cd2 100644 > --- a/chardev/char.c > +++ b/chardev/char.c > @@ -1028,23 +1028,10 @@ Chardev *qemu_chardev_new(const char *id, const char *typename, > return chr; > } > > -ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend, > - Error **errp) > +static ChardevReturn *chardev_add(const char *id, Chardev *chr, > + Error **errp) > { > - const ChardevClass *cc; > ChardevReturn *ret; > - Chardev *chr; > - > - cc = char_get_class(ChardevBackendKind_str(backend->type), errp); > - if (!cc) { > - return NULL; > - } > - > - chr = chardev_new(id, object_class_get_name(OBJECT_CLASS(cc)), > - backend, NULL, false, errp); > - if (!chr) { > - return NULL; > - } > > if (!object_property_try_add_child(get_chardevs_root(), id, OBJECT(chr), > errp)) { > @@ -1062,6 +1049,26 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend, > return ret; > } > > +ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend, > + Error **errp) > +{ > + const ChardevClass *cc; > + Chardev *chr; > + > + cc = char_get_class(ChardevBackendKind_str(backend->type), errp); > + if (!cc) { > + return NULL; > + } > + > + chr = chardev_new(id, object_class_get_name(OBJECT_CLASS(cc)), > + backend, NULL, false, errp); > + if (!chr) { > + return NULL; > + } > + > + return chardev_add(id, chr, errp); > +} > + > ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, > Error **errp) > { > @@ -1070,7 +1077,6 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, > Chardev *chr, *chr_new; > bool closed_sent = false; > bool handover_yank_instance; > - ChardevReturn *ret; > > chr = qemu_chr_find(id); > if (!chr) { > @@ -1089,11 +1095,22 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, > return NULL; > } > > + cc = CHARDEV_GET_CLASS(chr); > + cc_new = char_get_class(ChardevBackendKind_str(backend->type), errp); > + if (!cc_new) { > + return NULL; > + } > + > be = chr->be; > if (!be) { > /* easy case */ > - object_unparent(OBJECT(chr)); > - return qmp_chardev_add(id, backend, errp); > + chr_new = chardev_new(id, object_class_get_name(OBJECT_CLASS(cc_new)), > + backend, NULL, false, errp); > + if (!chr_new) { > + return NULL; > + } > + > + goto out; > } > > if (!be->chr_be_change) { > @@ -1101,12 +1118,6 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, > return NULL; > } > > - cc = CHARDEV_GET_CLASS(chr); > - cc_new = char_get_class(ChardevBackendKind_str(backend->type), errp); > - if (!cc_new) { > - return NULL; > - } > - > /* > * The new chardev should not register a yank instance if the current > * chardev has registered one already. > @@ -1147,18 +1158,9 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, > */ > chr->handover_yank_instance = handover_yank_instance; > > +out: > object_unparent(OBJECT(chr)); > - object_property_add_child(get_chardevs_root(), chr_new->label, > - OBJECT(chr_new)); > - object_unref(OBJECT(chr_new)); > - > - ret = g_new0(ChardevReturn, 1); > - if (CHARDEV_IS_PTY(chr_new)) { > - ret->pty = g_strdup(chr_new->filename + 4); > - ret->has_pty = true; > - } > - > - return ret; > + return chardev_add(id, chr_new, errp); > } > > void qmp_chardev_remove(const char *id, Error **errp) > -- > 2.25.1 >
diff --git a/chardev/char.c b/chardev/char.c index d959eec522..cb6f287cd2 100644 --- a/chardev/char.c +++ b/chardev/char.c @@ -1028,23 +1028,10 @@ Chardev *qemu_chardev_new(const char *id, const char *typename, return chr; } -ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend, - Error **errp) +static ChardevReturn *chardev_add(const char *id, Chardev *chr, + Error **errp) { - const ChardevClass *cc; ChardevReturn *ret; - Chardev *chr; - - cc = char_get_class(ChardevBackendKind_str(backend->type), errp); - if (!cc) { - return NULL; - } - - chr = chardev_new(id, object_class_get_name(OBJECT_CLASS(cc)), - backend, NULL, false, errp); - if (!chr) { - return NULL; - } if (!object_property_try_add_child(get_chardevs_root(), id, OBJECT(chr), errp)) { @@ -1062,6 +1049,26 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend, return ret; } +ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend, + Error **errp) +{ + const ChardevClass *cc; + Chardev *chr; + + cc = char_get_class(ChardevBackendKind_str(backend->type), errp); + if (!cc) { + return NULL; + } + + chr = chardev_new(id, object_class_get_name(OBJECT_CLASS(cc)), + backend, NULL, false, errp); + if (!chr) { + return NULL; + } + + return chardev_add(id, chr, errp); +} + ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, Error **errp) { @@ -1070,7 +1077,6 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, Chardev *chr, *chr_new; bool closed_sent = false; bool handover_yank_instance; - ChardevReturn *ret; chr = qemu_chr_find(id); if (!chr) { @@ -1089,11 +1095,22 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, return NULL; } + cc = CHARDEV_GET_CLASS(chr); + cc_new = char_get_class(ChardevBackendKind_str(backend->type), errp); + if (!cc_new) { + return NULL; + } + be = chr->be; if (!be) { /* easy case */ - object_unparent(OBJECT(chr)); - return qmp_chardev_add(id, backend, errp); + chr_new = chardev_new(id, object_class_get_name(OBJECT_CLASS(cc_new)), + backend, NULL, false, errp); + if (!chr_new) { + return NULL; + } + + goto out; } if (!be->chr_be_change) { @@ -1101,12 +1118,6 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, return NULL; } - cc = CHARDEV_GET_CLASS(chr); - cc_new = char_get_class(ChardevBackendKind_str(backend->type), errp); - if (!cc_new) { - return NULL; - } - /* * The new chardev should not register a yank instance if the current * chardev has registered one already. @@ -1147,18 +1158,9 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, */ chr->handover_yank_instance = handover_yank_instance; +out: object_unparent(OBJECT(chr)); - object_property_add_child(get_chardevs_root(), chr_new->label, - OBJECT(chr_new)); - object_unref(OBJECT(chr_new)); - - ret = g_new0(ChardevReturn, 1); - if (CHARDEV_IS_PTY(chr_new)) { - ret->pty = g_strdup(chr_new->filename + 4); - ret->has_pty = true; - } - - return ret; + return chardev_add(id, chr_new, errp); } void qmp_chardev_remove(const char *id, Error **errp)
To improve the problematic source code in qmp_chardev_change and some redundant source code, the functions qmp_chardev_add and qmp_chardev_change are refactored. It is mentioned in thread: https://lists.nongnu.org/archive/html/qemu-devel/2021-04/msg03167.html Signed-off-by: Li Zhang <li.zhang@ionos.com> --- chardev/char.c | 72 ++++++++++++++++++++++++++------------------------ 1 file changed, 37 insertions(+), 35 deletions(-)