virt-aa-helper: Misc improvements

[PATCH 01/15] log_cleaner: Use virFileCanonicalizePath()

Posted by Michal Privoznik via Devel 2 months, 4 weeks ago

From: Michal Privoznik <mprivozn@redhat.com>

While use of realpath() is not forbidden, our some of our mocks
already have a test friendly reimplementation of
virFileCanonicalizePath(). Use the latter.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/logging/log_cleaner.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/logging/log_cleaner.c b/src/logging/log_cleaner.c
index 4efcbc18e4..d247fdf829 100644
--- a/src/logging/log_cleaner.c
+++ b/src/logging/log_cleaner.c
@@ -66,7 +66,7 @@ virLogCleanerParseFilename(const char *path,
     g_autofree char *clear_path = NULL;
     char *chain_prefix = NULL;
 
-    clear_path = realpath(path, NULL);
+    clear_path = virFileCanonicalizePath(path);
     if (!clear_path) {
         VIR_WARN("Failed to resolve path %s: %s", path, g_strerror(errno));
         return NULL;
-- 
2.49.0

Re: [PATCH 01/15] log_cleaner: Use virFileCanonicalizePath()

Posted by Ján Tomko via Devel 2 months, 1 week ago

On a Thursday in 2025, Michal Privoznik via Devel wrote:
>From: Michal Privoznik <mprivozn@redhat.com>
>
>While use of realpath() is not forbidden, our some of our mocks
>already have a test friendly reimplementation of
>virFileCanonicalizePath(). Use the latter.
>

That's a good enough reason to forbid it in syntax-check.

In fact, the use of realpath in virFilleCanonicalizePath already
has a comment it's exempt from syntax-check. There is no rule for it,
because the comment is a leftover from when it used
canonicalize_file_name:

commit b4d601ba87ade7fa1a3a4f9c0c268659c15a35c3
     util: use realpath/g_canonicalize_filename

The syntax-check rule was specifically introduced because of mocking:
commit 00d465bb4df4a49e07d51ce289142b5fae8ba344
     syntax-check: Prohibit canonicalize_file_name()

Jano

>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/logging/log_cleaner.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/src/logging/log_cleaner.c b/src/logging/log_cleaner.c
>index 4efcbc18e4..d247fdf829 100644
>--- a/src/logging/log_cleaner.c
>+++ b/src/logging/log_cleaner.c
>@@ -66,7 +66,7 @@ virLogCleanerParseFilename(const char *path,
>     g_autofree char *clear_path = NULL;
>     char *chain_prefix = NULL;
>
>-    clear_path = realpath(path, NULL);
>+    clear_path = virFileCanonicalizePath(path);
>     if (!clear_path) {
>         VIR_WARN("Failed to resolve path %s: %s", path, g_strerror(errno));
>         return NULL;
>-- 
>2.49.0
>

Re: [PATCH 01/15] log_cleaner: Use virFileCanonicalizePath()

Posted by Michal Prívozník via Devel 2 months, 1 week ago

On 7/2/25 12:22, Ján Tomko wrote:
> On a Thursday in 2025, Michal Privoznik via Devel wrote:
>> From: Michal Privoznik <mprivozn@redhat.com>
>>
>> While use of realpath() is not forbidden, our some of our mocks
>> already have a test friendly reimplementation of
>> virFileCanonicalizePath(). Use the latter.
>>
> 
> That's a good enough reason to forbid it in syntax-check.

Sure, let me post it in a follow up patch.

> 
> In fact, the use of realpath in virFilleCanonicalizePath already
> has a comment it's exempt from syntax-check. There is no rule for it,
> because the comment is a leftover from when it used
> canonicalize_file_name:
> 
> commit b4d601ba87ade7fa1a3a4f9c0c268659c15a35c3
>     util: use realpath/g_canonicalize_filename
> 
> The syntax-check rule was specifically introduced because of mocking:
> commit 00d465bb4df4a49e07d51ce289142b5fae8ba344
>     syntax-check: Prohibit canonicalize_file_name()
> 
> Jano
> 
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>> src/logging/log_cleaner.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/logging/log_cleaner.c b/src/logging/log_cleaner.c
>> index 4efcbc18e4..d247fdf829 100644
>> --- a/src/logging/log_cleaner.c
>> +++ b/src/logging/log_cleaner.c
>> @@ -66,7 +66,7 @@ virLogCleanerParseFilename(const char *path,
>>     g_autofree char *clear_path = NULL;
>>     char *chain_prefix = NULL;
>>
>> -    clear_path = realpath(path, NULL);
>> +    clear_path = virFileCanonicalizePath(path);
>>     if (!clear_path) {
>>         VIR_WARN("Failed to resolve path %s: %s", path,
>> g_strerror(errno));
>>         return NULL;
>> -- 
>> 2.49.0
>>

Michal