On Mon, Jul 23, 2018 at 01:19:49PM +0200, Erik Skultety wrote:
> On Sat, Jul 21, 2018 at 05:36:35PM +0530, Sukrit Bhatnagar wrote:
> > Create typedefs virBufferEscapePair and virBufferEscapePairPtr
> > for struct _virBufferEscapePair for cleaner code and for use
> > with cleanup macros.
> >
> > Also create a dedicated Free helper virBufferEscapePairFree.
>
> Usually, a sentence like ^this indicates, that the change itself should come
> separately.
>
> >
> > Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@xxxxxxxxx>
> > ---
> > src/util/virbuffer.c | 16 ++++++++++++++--
> > 1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
> > index 3d6defb..8076cd3 100644
> > --- a/src/util/virbuffer.c
> > +++ b/src/util/virbuffer.c
> > @@ -648,11 +648,23 @@ virBufferEscape(virBufferPtr buf, char escape, const char *toescape,
> > }
> >
> >
> > +typedef struct _virBufferEscapePair virBufferEscapePair;
> > +typedef virBufferEscapePair *virBufferEscapePairPtr;
>
> ^This should come as a separate patch.
>
> > +
> > struct _virBufferEscapePair {
> > char escape;
> > char *toescape;
> > };
> >
> > +static void
> > +virBufferEscapePairFree(virBufferEscapePairPtr pair)
> > +{
> > + if (!pair)
> > + return;
> > +
> > + VIR_FREE(pair);
> > +}
>
> Patch doesn't compile, please make sure that each patch can pass
> make check && make syntax-check.
>
> ^This function should be introduced in the following patch.
An idea to consider would be to do VIR_STRDUP on the toescape strings, so that
this free helper would do VIR_FREE(toescape), that might make more sense in
terms of logic. It's also safer, since we can't predict the lifetime of the
string passed by the caller, they might as well free it immediately after
returning from a helper, assuming the helper created their own copy of the
string.
Erik
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
