[libvirt PATCH] tests: Use freecon in testSELinuxCheckLabels
This partially reverts commit 0fc4a43d248b86fd54ad7323beb66faec8c1043c. Signed-off-by: Jiri Denemark <jdenemar@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 @@ -230,7 +230,7 @@ testSELinuxCheckLabels(testSELinuxFile *files, size_t nfiles) size_t i; for (i = 0; i < nfiles; i++) { - g_autofree char *ctx = NULL; + char *ctx = NULL; if (getfilecon(files[i].file, &ctx) < 0) { if (errno == ENODATA) { /* nothing to do */ @@ -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; } -- 2.35.0
On Fri, Feb 11, 2022 at 14:34:04 +0100, Jiri Denemark wrote:
This partially reverts commit 0fc4a43d248b86fd54ad7323beb66faec8c1043c.
Signed-off-by: Jiri Denemark <jdenemar@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 @@ -230,7 +230,7 @@ testSELinuxCheckLabels(testSELinuxFile *files, size_t nfiles) size_t i;
for (i = 0; i < nfiles; i++) { - g_autofree char *ctx = NULL; + char *ctx = NULL; if (getfilecon(files[i].file, &ctx) < 0) { if (errno == ENODATA) { /* nothing to do */ @@ -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.
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@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. Jano [0] https://github.com/SELinuxProject/selinux/blob/master/libselinux/src/freecon...
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@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...
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
On Fri, Feb 11, 2022 at 03:16:35PM +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@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?
SELinux uses to use 'security_context_t' data type for all its APIs, whicih was a typedef for 'char *'. They stopped doing that and now just use 'char *'. The docs for freecon still say it is to be used to free memory from anmy other get* function, but impl is just delegating to free() Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (3)
-
Daniel P. Berrangé -
Jiri Denemark -
Ján Tomko