[pacman-dev] [PATCH 1/2] Make sync DB reading a bit more flexible

[Dan McGee]

On Mon, Jun 27, 2011 at 4:36 AM, Allan McRae <allan at archlinux.org> wrote:
> On 24/06/11 15:18, Dan McGee wrote:
>>
>> We can reorganize things a bit to not require reading a directory-only
>> entry
>> first (or at all). This was noticed while working on some pactest
>> improvements, but should be a good step forward anyway.
>>
>> Also make _alpm_splitname() a bit more generic in where it stores the data
>> it
>> parses.
>>
>> Signed-off-by: Dan McGee<dan at archlinux.org>
>
> Signed-off-by: Allan with minor query below.
>
>> ---
>>  lib/libalpm/be_local.c |    3 +-
>>  lib/libalpm/be_sync.c  |  130
>> ++++++++++++++++++++++++-----------------------
>>  lib/libalpm/util.c     |   46 ++++++++++-------
>>  lib/libalpm/util.h     |    3 +-
>>  4 files changed, 97 insertions(+), 85 deletions(-)
>>
>
> <snip>
> All the be_sync.c stuff looks fine to me.
>
>>
>> diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
>> index 4976703..e56c9f2 100644
>> --- a/lib/libalpm/util.c
>> +++ b/lib/libalpm/util.c
>> @@ -825,46 +825,54 @@ cleanup:
>>        }
>>  }
>>
>> -int _alpm_splitname(const char *target, pmpkg_t *pkg)
>> +int _alpm_splitname(const char *target, char **name, char **version,
>> +               unsigned long *name_hash)
>>  {
>>        /* the format of a db entry is as follows:
>>         *    package-version-rel/
>> +        *    package-version-rel/desc (we ignore the filename portion)
>>         * package name can contain hyphens, so parse from the back- go
>> back
>>         * two hyphens and we have split the version from the name.
>>         */
>> -       const char *version, *end;
>> +       const char *pkgver, *end;
>>
>> -       if(target == NULL || pkg == NULL) {
>> +       if(target == NULL) {
>>                return -1;
>>        }
>
> Should we check name/version/name_hash here for being NULL and abort similar
> to what was done for pkg previously?  Or could it be of use for this
> function to be called with those as NULL?
I didn't null check here because
1) Look later in the function; we only set version and name if they are non-NULL
2) in reality, the current two callers guarantee non-NULL for those
arguments, so even that might be overkill.

-Dan