From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 69837 invoked by alias); 18 Oct 2017 15:28:42 -0000 Mailing-List: contact cygwin-apps-help@cygwin.com; run by ezmlm Precedence: bulk Sender: cygwin-apps-owner@cygwin.com List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Mail-Followup-To: cygwin-apps@cygwin.com Received: (qmail 69827 invoked by uid 89); 18 Oct 2017 15:28:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=our X-HELO: limerock04.mail.cornell.edu Received: from limerock04.mail.cornell.edu (HELO limerock04.mail.cornell.edu) (128.84.13.244) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 18 Oct 2017 15:28:40 +0000 X-CornellRouted: This message has been Routed already. Received: from authusersmtp.mail.cornell.edu (granite3.serverfarm.cornell.edu [10.16.197.8]) by limerock04.mail.cornell.edu (8.14.4/8.14.4_cu) with ESMTP id v9IFSb9p017619 for ; Wed, 18 Oct 2017 11:28:38 -0400 Received: from [192.168.0.15] (mta-68-175-129-7.twcny.rr.com [68.175.129.7] (may be forged)) (authenticated bits=0) by authusersmtp.mail.cornell.edu (8.14.4/8.12.10) with ESMTP id v9IFSaW6019329 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT) for ; Wed, 18 Oct 2017 11:28:37 -0400 Subject: Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP) To: cygwin-apps@cygwin.com References: <20170531105015.162228-1-jon.turney@dronecode.org.uk> <488ba627-de58-ddc7-7f69-696adae76b8a@cornell.edu> <9bcf50cf-81bc-c9d1-3ac3-b7e1a3522045@dronecode.org.uk> <5441628f-a99a-1611-616a-da98ea9a0e12@cornell.edu> <7044db65-8b6e-6bf6-a079-99397917ce43@cornell.edu> <8e655423-ef53-3aeb-3d6c-de5021d3bd87@dronecode.org.uk> <5e74cacd-3153-1561-3cd2-5ece0e35a2d2@cornell.edu> <003a8566-ad2b-a962-725a-4384fd5e4c64@cornell.edu> From: Ken Brown Message-ID: <02e7c6e5-3f7b-5e43-1557-8031b7bb1535@cornell.edu> Date: Wed, 18 Oct 2017 15:28:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-PMX-Cornell-Gauge: Gauge=XXXXX X-PMX-CORNELL-AUTH-RESULTS: dkim-out=none; X-IsSubscribed: yes X-SW-Source: 2017-10/txt/msg00070.txt.bz2 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