<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">On 13/01/22 6:47 pm, Ani Sinha wrote:<br>
</div>
<blockquote type="cite" cite="mid:CAARzgwzZtfQa1OQ6pt6B0ukQT69tuPnfB4ocwQNracZq2_K54A@mail.gmail.com">
<div><br>
</div>
<div><br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Thu, Jan 13, 2022 at 4:50
PM John Levon <<a href="mailto:levon@movementarian.org" moz-do-not-send="true" class="moz-txt-link-freetext">levon@movementarian.org</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left-width:1px;border-left-style:solid;padding-left:1ex;border-left-color:rgb(204,204,204)">On
Thu, Jan 13, 2022 at 03:11:51PM +0530, Ani Sinha wrote:<br>
<br>
> > diff --git a/src/qemu/qemu_command.c
b/src/qemu/qemu_command.c<br>
> > index d822533ccb..4130df0ed9 100644<br>
> > --- a/src/qemu/qemu_command.c<br>
> > +++ b/src/qemu/qemu_command.c<br>
> > @@ -10718,6 +10718,8 @@
qemuBuildSerialChrDeviceProps(const virDomainDef *def,<br>
> > g_autoptr(virJSONValue) props = NULL;<br>
> > g_autofree char *chardev =
g_strdup_printf("char%s", serial->info.alias);<br>
> > virQEMUCapsFlags caps;<br>
> > + const char *typestr;<br>
> > + int ret;<br>
> <br>
> type should match the return type of this function.<br>
<br>
It does match return type of
virDomainChrSerialTargetModelTypeToString():<br>
<br>
47 #define VIR_ENUM_DECL(name) \
<br>
48 const char *name ## TypeToString(int type); \
</blockquote>
<div dir="auto"><br>
</div>
<div dir="auto">Yes you are correct. I was thinking of <span style="word-spacing:1px;color:rgb(49,49,49)">qemuBuildSerialChrDeviceProps()
but it does not return ret directly from this function.
Another reason it makes me uncomfortable. </span></div>
<div dir="auto"><span style="word-spacing:1px;color:rgb(49,49,49)"><br>
</span></div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left-width:1px;border-left-style:solid;padding-left:1ex;border-left-color:rgb(204,204,204)" dir="auto"><br>
> I preferred your previous style to this one.<br>
<br>
Below is cleaner IMO: we don't repeat code, and the flow is
much clearer.</blockquote>
<div dir="auto"><br>
</div>
<div dir="auto">If you look at other functions for example <span style="color:rgb(0,0,0)">qemuBuildVirtioDevProps() etc </span>they
follow the previous style. I think it's fine to return NULL
from multiple points.</div>
<div dir="auto">In any case if the maintainers are fine with
it , I'm ok. Style is trivial.</div>
</div>
</div>
</blockquote>
<p>Thankyou Ani ! Let me know if I need to update anything else for
the patch. <br>
</p>
<p>Thanks :)<br>
</p>
</body>
</html>