Hello, Ok. Thx for review. I will try today make new function for it and also will look for how arguments should be splitted in multiple lines. -- Best regards / Pozdrawiam Sławek Kapłoński slawek@xxxxxxxxxxxx On Mon, 10 Oct 2016, Peter Krempa wrote: > On Mon, Oct 10, 2016 at 04:53:15 +0200, Michal Privoznik wrote: > > On 08.10.2016 23:38, Sławek Kapłoński wrote: > > > New line character in name of network is now forbidden because it > > > mess virsh output and can be confusing for users. > > > > > > Closes-Bug: https://bugzilla.redhat.com/show_bug.cgi?id=818064 > > > --- > > > src/conf/network_conf.c | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c > > > index aa39776..f9c537f 100644 > > > --- a/src/conf/network_conf.c > > > +++ b/src/conf/network_conf.c > > > @@ -2123,6 +2123,13 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) > > > goto error; > > > } > > > > > > + if (strchr(def->name, '\n')) { > > > + virReportError( > > > + VIR_ERR_XML_ERROR, > > > + _("name %s cannot contain new-line character"), def->name); > > Please note that this is not our expected style of breaking up > arguments. > > > > + goto error; > > > + } > > > + > > > /* Extract network uuid */ > > > tmp = virXPathString("string(./uuid[1])", ctxt); > > > if (!tmp) { > > > > > > > Okay, this makes sense. We can put reasonable restrictions on object > > names. However, I think while we are at this we can fix the same problem > > in other areas of the code. For instance, storage pools and volumes > > suffer from the same problem. Now, instead of copying this chunk all > > over the places, what about doing this more wisely? > > > > For instance: > > > > char *c; > > ... > > /* Parse name from XML */ > > > > if ((c = strpbrk(def->name, "/\n")) { > > /* Insert your favourite character here ^^ */ > > virReportError(VIR_ERR_XML_DETAIL, > > _("invalid char in name: %c"), *c); > > return -1; /* goto cleanup */ > > } > > > > This way, we can just add new character to the string when somebody > > files a bug. > > Moreover, if we'd wrap those couple of lines into a function, we could > > just do: > > > > if (!virXMLCheckName(def->name, "/\n")) > > return -1; > > > > Let me know if you have any troubles writing this, or if you disagree. > > The problem with making the parser stricter is that it's prone to create > regressions for people that might have used such configuration. > > As this was previously working (except for the broken virsh interface) > we should limit such names only when defining a network/vm/whatever. > > Peter > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list