From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web11.88225.1597844209241509062 for ; Wed, 19 Aug 2020 06:36:49 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=RyWkTKzk; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1597844207; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=u1vLA6XsOsnJSf7FamGovo9GIzhaveUyurAUYfJcqKQ=; b=RyWkTKzkaMXhNxyiSOB5q1RtPfweLwAnURIowo3hIxK3eiu4/REZQzLOPnRH1+GpHVJQba WCPTIN7+InG7bKJTlL+U3D3FLmMzGDFSBEooDNVLCvz8y/rI9bLJxNLnncl2UXWjgMzxN7 R80DR2glHS06dN+4MjKpEFD0A0SyJTo= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-281-9I8_MHaeNGOz0qLdtMYX2w-1; Wed, 19 Aug 2020 09:36:43 -0400 X-MC-Unique: 9I8_MHaeNGOz0qLdtMYX2w-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 6CA94640C8; Wed, 19 Aug 2020 13:36:42 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-57.ams2.redhat.com [10.36.114.57]) by smtp.corp.redhat.com (Postfix) with ESMTP id B0C647DFDD; Wed, 19 Aug 2020 13:36:40 +0000 (UTC) Subject: Re: [edk2-announce] Re: Soft Feature Freeze starts now for edk2-stable202008 From: "Laszlo Ersek" To: Leif Lindholm , "Chang, Abner (HPS SW/FW Technologist)" Cc: "devel@edk2.groups.io" , "liming.gao" , "announce@edk2.groups.io" , "afish@apple.com" , "Kinney, Michael D" References: <877468e5-d154-14fc-b23b-ffa8fd2c9103@redhat.com> <03b3deed-8506-94c3-d14a-eca9198b48a2@redhat.com> <20200819114853.GF17439@vanye> <7fa9c99d-56d1-842e-3fd4-2d3fe649b588@redhat.com> Message-ID: <925886ad-da8e-6d9f-b2d5-ae59600938fd@redhat.com> Date: Wed, 19 Aug 2020 15:36:39 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <7fa9c99d-56d1-842e-3fd4-2d3fe649b588@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0.001 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 08/19/20 15:19, Laszlo Ersek wrote: > On 08/19/20 13:48, Leif Lindholm wrote: >> (Slightly trimmed recipient list due to different patch being discussed.) >> >> So, I can't make this call, because I'm the one who messed up. >> >> This patch does exactly what I had requested Abner to do some time >> back (off-list, unfortunately), and I was *convinced* I gave it an R-b >> as soon as it hit my inbox - until Abner nudged me about it yesterday. >> >> The patch in question is >> https://edk2.groups.io/g/devel/topic/76021725 > > My understanding is: > > (1) there is an external project that consumes the FDT library in > EmbeddedPkg, meaning the lib class header "EmbeddedPkg/Include/libfdt.h" > and the lib instance "EmbeddedPkg/Library/FdtLib/FdtLib.inf", > > (2) the lib class header pulls in "fdt.h" and "libfdt_env.h", > > (3) the external project is not edk2-platforms, > > (4) the external project wants -- for some strange reason -- edk2's > "libfdt_env.h" to provide an strncmp() function (or function-like > macro), with that particular stncmp() implementation not being needed in > either edk2-platforms or edk2 itself, > > (5) the patch for adding said strncmp() was posted on Aug 6th (at least > when viewed from my time zone), i.e., before the SFF, > > (6) it was reviewed 12 days later (within the SFF) > > If my understanding is correct, then I don't see how this patch could be > considered a bugfix -- even as a feature addition, it seems hardly > justified to me --, and there would have been ~8 days before the SFF to > review it. > > I think we should postpone the patch until after the stable tag. Regarding OpenSBI, as of commit 9d56961b2314, the sbi_strncmp() function has not been removed, and it seems that sbi_strncmp() is still used / called in at least some build configurations (where the lib/utils/libfdt/libfdt_env.h:#define strncmp sbi_strncmp definition is supposed to be in effect). This means that the codebase can not rid itself of sbi_strncmp(). To me this indicates that OpenSBI commit 2cfd2fc90488 ("lib: utils: Use strncmp in fdt_parse_hart_id()", 2020-07-29) was wrong. It should have replaced sbi_strcmp() with sbi_strncmp(), not strncmp(). After all, the size limit seems to be the motivation for the entire change -- put a size limit on the string comparison in fdt_parse_hart_id(). Commit 2cfd2fc90488 did two things at the same time: replace the size-unbounded comparison with the size-bounded one, *and* switch to the standard C function name from the self-contained function. The former goal looks justifiable, the latter I don't understand. Now: I realize that, going back to edk2 commit fa4a70633868 ("EmbeddedPkg: Added support to libfdt", 2012-09-27), we already have a bunch of wrappers in "EmbeddedPkg/Include/libfdt_env.h". So I guess adding "one more" is not inconsistent with those -- even though the lib instance within edk2 doesn't need the new function. But it's still a micro-feature whose review should have completed before the SFF. Thanks Laszlo