Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown <kbrown@cornell.edu>]

On 10/17/2017 2:46 PM, Jon Turney wrote:
> On 17/10/2017 13:44, Ken Brown wrote:
>> On 10/10/2017 7:18 AM, Ken Brown wrote:
>>> On 9/29/2017 4:33 PM, Ken Brown wrote:
>>>> I'll resume my testing after I return.
>>>
>>> I've just started testing (based on the current HEAD of 
>>> topic/libsolv), and so far everything looks good.
>>
>> I came across a situation where a SolvableVersion method was being 
>> called on a trivial object (with pool and id both 0).  This caused a 
>> crash when pool_id2solvable(pool, id) was called and pool was 
>> dereferenced.  There's probably a bug that led to this situation.  [It 
>> involved a local install in which a package was listed in two 
>> different setup.ini files, but the tarballs existed only in one.]  I 
>> plan to investigate this further.  But in any case, we shouldn't 
>> crash.  Patch attached.
> 
> I thought about putting this in, but decided against it as it would 
> probably catch some mistakes I had made...
> 
> But yeah, for production use, I think not crashing is probably a good 
> idea :).  Although I guess we might want asserts or something, if we 
> think these cases shouldn't be happening.

Here's the situation where I got the crash: Package A is installed and 
requires B.  setup tries to install B, but the install fails for some 
reason.  During the postinstall phase, we're scanning the dependencies 
of A and we call checkForInstalled to see if B is installed.  This ends 
up calling PackageSpecification::satisfies(aPackage), with aPackage 
being the empty package (pool == NULL, id == 0) because B is not 
installed.  A call to aPackage.Name() then causes the crash.

I think this sequence of calls is reasonable.  Name() is part of the 
public interface to SolvableVersion, so we should be able to call Name() 
on any package without a crash or assertion violation.  In the case of 
the empty package, the empty string is a reasonable return value.

Similar considerations apply to the other public member functions of 
SolvableVersion.  So my inclination is to go with something like my 
patch rather than changing all callers.

Ken