[pacman-dev] [PATCH 3/3] pacman: add file checksum validation against mtree

[Allan McRae]

On 5/1/21 12:04 am, Emil Velikov via pacman-dev wrote:
> On Tue, 29 Dec 2020 at 03:02, Allan McRae <allan at archlinux.org> wrote:
>>
>> On 24/12/20 8:43 am, Emil Velikov wrote:
>>> With libarchive v3.5.0 we have API to fetch the digest from the mtree.
>>> Use that to validate if the installed files are modified or not.
>>>
>>> As always, a modified backup file will trigger a warning but will not
>>> result in an actual failure.
>>>
>>> TODO: localization... no idea how that is even done :-)
>>
>> Adding the _() around the strings is all that needed done.
>>
> Perfect thanks.
> 
>>> NOTE: indentation is likely all over the place - first time I see ts=2
>>
>> For line wraps, we generally just use two indents, so really does not
>> matter what the tab system is.
>>
> Was mostly curious about wrapping of multiline function calls. It
> seems that "use an extra tab" is common practise - worth adding an
> example in the Coding style section in HACKING?
> 
>>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>>> ---
>>>  src/pacman/check.c | 66 +++++++++++++++++++++++++++++++++++++++++-----
>>>  1 file changed, 59 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/pacman/check.c b/src/pacman/check.c
>>> index 02217d0f..083c547d 100644
>>> --- a/src/pacman/check.c
>>> +++ b/src/pacman/check.c
>>> @@ -176,19 +176,70 @@ static int check_file_size(const char *pkgname, const char *filepath,
>>>       return 0;
>>>  }
>>>
>>> -/* placeholders - libarchive currently does not read checksums from mtree files
>>> -static int check_file_md5sum(const char *pkgname, const char *filepath,
>>> -             struct stat *st, struct archive_entry *entry, int backup)
>>> +#if ARCHIVE_VERSION_NUMBER >= 3005000
>>
>> This does not need wrapped in #if.  There is nothing libarchive specific
>> in this function.
>>
> Without this the compiler will flag it as -Wunused-function. I could
> silence that by annotating the function as __attribute__((used)).
> Would you prefer that or you have something else in mind?
> 

Ah - missed that.  Keep the #if then

A

>>> +static int check_file_cksum(const char *pkgname, const char *filepath,
>>> +             int backup, const char *cksum_name, const char *cksum_calc, const char *cksum_mtree)
>>>  {
>>> +     if(!cksum_calc || !cksum_mtree) {
>>
>> Only one of these failures matches the error message.   Split into
>> "checkusm information not available" and "failed to calculate checksum"
>>
> Ack will do.
> 
> Thanks
> Emil
> .
>