27 Jun
2011
27 Jun
'11
12:16 p.m.
On 27/06/11 22:04, Dan McGee wrote:
> On Mon, Jun 27, 2011 at 4:36 AM, Allan McRae<allan@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@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