[PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)
Ken Brown
kbrown@cornell.edu
Wed Oct 18 15:28:00 GMT 2017
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
More information about the Cygwin-apps
mailing list