[libvirt PATCH] tests: Use freecon in testSELinuxCheckLabels

Jiri Denemark jdenemar at redhat.com
Fri Feb 11 14:23:46 UTC 2022


On Fri, Feb 11, 2022 at 15:16:35 +0100, Ján Tomko wrote:
> On a Friday in 2022, Jiri Denemark wrote:
> >On Fri, Feb 11, 2022 at 14:34:04 +0100, Jiri Denemark wrote:
> >> This partially reverts commit 0fc4a43d248b86fd54ad7323beb66faec8c1043c.
> >>
> >> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> >> ---
> >>  tests/securityselinuxlabeltest.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c
> >> index dfe9d38d34..83532d8090 100644
> >> --- a/tests/securityselinuxlabeltest.c
> >> +++ b/tests/securityselinuxlabeltest.c
> >> @@ -247,8 +247,10 @@ testSELinuxCheckLabels(testSELinuxFile *files, size_t nfiles)
> >>              virReportError(VIR_ERR_INTERNAL_ERROR,
> >>                             "File %s context '%s' did not match expected '%s'",
> >>                             files[i].file, ctx, files[i].context);
> >> +            freecon(ctx);
> >>              return -1;
> >>          }
> >> +        freecon(ctx);
> >>      }
> >>      return 0;
> >>  }
> >
> >Self-NACK
> >
> >getfilecon is mocked in this test so not using freecon() was actually
> >correct.
> >
> 
> Even if it was from libselinux [0], is there something wrong with using
> plain free for a char * variable?
> 
> We seem to have exactly one user of xmlFree which is a similar function.
> 
> [0] https://github.com/SELinuxProject/selinux/blob/master/libselinux/src/freecon.c#L6

Well, not really, unless the library decides to use a different
allocator which it easily can since the documentation says you should be
using freecon(). And we use freecon in other places in our code to
comply with this.

Anyway, I really wish libraries did not invent their own free functions
for char *.

Jirka




More information about the libvir-list mailing list