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

[Allan McRae]

On 27/06/11 22:04, Dan McGee wrote:
> 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.

Sure.  If they were checks for NULL there, then checks later in the 
function could obviously be removed.  But I suppose that one day we 
might like to use this just to get the name so version and name_hash 
could be passed as NULL.

So ack from me.  BTW, the patch needed a minor rebase to apply to the 
current head.  That is done on my working branch.

Allan