On 1/7/25 01:56, Jim Fehlig via Devel wrote:
> On 11/13/24 07:28, Georgia Garcia wrote:
>> Moving towards full adoption of GLib APIs in the AppArmor code.
>>
>> Signed-off-by: Georgia Garcia <georgia.garcia@xxxxxxxxxxxxx>
>> ---
>> src/security/security_apparmor.c | 41 ++++---------
>> src/security/virt-aa-helper.c | 100 ++++++++++---------------------
>> 2 files changed, 45 insertions(+), 96 deletions(-)
>>
>> diff --git a/src/security/security_apparmor.c b/src/security/
>> security_apparmor.c
>> index 7092724563..9e578b2526 100644
>> --- a/src/security/security_apparmor.c
>> +++ b/src/security/security_apparmor.c
>> @@ -115,37 +115,28 @@ profile_loaded(const char *str)
>> static int
>> profile_status_file(const char *str)
>> {
>> - char *profile = NULL;
>> - char *content = NULL;
>> - char *tmp = NULL;
>> - int rc = -1;
>> + g_autofree char *profile = NULL;
>> + g_autofree char *content = NULL;
>> + g_autofree char *tmp = NULL;
>> int len;
>> profile = g_strdup_printf("%s/%s", APPARMOR_DIR "/libvirt", str);
>> if (!virFileExists(profile))
>> - goto failed;
>> + return -1;
>> if ((len = virFileReadAll(profile, MAX_FILE_LEN, &content)) <
>> 0) {
>> virReportSystemError(errno,
>> _("Failed to read \'%1$s\'"), profile);
>> - goto failed;
>> + return -1;
>> }
>> /* create string that is ' <str> flags=(complain)\0' */
>> tmp = g_strdup_printf(" %s flags=(complain)", str);
>> if (strstr(content, tmp) != NULL)
>> - rc = 0;
>> - else
>> - rc = 1;
>> -
>> - failed:
>> - VIR_FREE(tmp);
>> - VIR_FREE(profile);
>> - VIR_FREE(content);
>> -
>> - return rc;
>> + return 0;
>> + return 1;
>> }
>> /*
>> @@ -218,7 +209,7 @@ static int
>> use_apparmor(void)
>> {
>> int rc = -1;
>> - char *libvirt_daemon = NULL;
>> + g_autofree char *libvirt_daemon = NULL;
>> if (virFileResolveLink("/proc/self/exe", &libvirt_daemon) < 0) {
>> virReportError(VIR_ERR_INTERNAL_ERROR,
>> @@ -232,7 +223,7 @@ use_apparmor(void)
>> return 1;
>> if (access(APPARMOR_PROFILES_PATH, R_OK) != 0)
>> - goto cleanup;
>> + return rc;
>> /* First check profile status using full binary path. If that
>> fails
>> * check using profile name.
>> @@ -245,8 +236,6 @@ use_apparmor(void)
>> rc = -1;
>> }
>> - cleanup:
>> - VIR_FREE(libvirt_daemon);
>> return rc;
>> }
>> @@ -948,7 +937,7 @@ AppArmorSetChardevLabel(virSecurityManager *mgr,
>> virDomainChrSourceDef *dev_source,
>> bool chardevStdioLogd G_GNUC_UNUSED)
>> {
>> - char *in = NULL, *out = NULL;
>> + g_autofree char *in = NULL, *out = NULL;
>
> While touching this code, we can follow the preference* of one variable
> per line. I can make that small tweak before pushing.
>
> Otherwise, the patch looks good!
>
> Reviewed-by: Jim Fehlig <jfehlig@xxxxxxxx>
>
> Regards,
> Jim
>
> * I _think_ that's the project preference, and vaguely recall others
> mentioning it in the past :-).
It is, because it leads to smaller diffs in the future. Please do make
the change before pushing these.
Michal
