From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1lGSii-0006Lg-Ls for mharc-grub-devel@gnu.org; Sun, 28 Feb 2021 15:31:24 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:37518) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lGSig-0006Kg-3l for grub-devel@gnu.org; Sun, 28 Feb 2021 15:31:14 -0500 Received: from mail-qk1-x72b.google.com ([2607:f8b0:4864:20::72b]:35909) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lGSiW-0004Qb-86 for grub-devel@gnu.org; Sun, 28 Feb 2021 15:31:13 -0500 Received: by mail-qk1-x72b.google.com with SMTP id n79so3017927qke.3 for ; Sun, 28 Feb 2021 12:31:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficientek-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references:reply-to :mime-version:content-transfer-encoding; bh=fxZp/JVhPKeie6pIO9sqk7KkgZY63thwqcWhoP1VNpY=; b=unhu3z+qCCTcEMraDSq9RamrYVLjTqsSI1R+yILMZoToIVYdMGIg2Qsyi65JKxnjSv 964S1odAMDGfHy95VupHyvpFnNXzSqTExjDvYfMSm9X2x/qSpeVi2WNjMFfV/sNXxuPY AKtfyLTNNuilwpAJs3BztooeIAjiaaJNmjYamQDwGQZjPUJWHQ8OzdsYRRD32rfPOPzD T0Q55S2A0aHwkam4I/2Iy/g5O7VBK9CjGt15SSDfc4aCecp7Mjhvcqqiq32RyXzaIME2 nKdJWQN+g7R7paoKkjZvcq/E/SQtPQNj4LDouOkHH9rRCeHmCUnUkfGQ4KN3DXEcAsOq TuOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:reply-to:mime-version:content-transfer-encoding; bh=fxZp/JVhPKeie6pIO9sqk7KkgZY63thwqcWhoP1VNpY=; b=hRTjQU/Ffqa+Uwybu3BrUPgo1CejHQ+50P5a+q7fFSd9wX9VCK6OsNdrDd+B6RAQyZ Z2gfsVPvbr94Sg7uqXpN4RrFiLCBCzyw58rTI+lFiJbXOXNNKg0hU3fqeyqiELiQhQIo dEwp8d8FOGnrnzUpdwlwmII1/cIoe7Nm7zGGuUx5YVacqVK76oLYANJF33327U58+oJ4 hYlMvMYsIWbZxR4lJYlbViOuUJBX95apLhTvZvdp67lPEJBD1TytDCmvHgG8AgQEbcoR +72C2JhuM3CU7AQ2XHIDLf1iTjlBwul4dwdDzR6eLXzfozP/SPOXzCQLlU6cEzhowB9c FE2g== X-Gm-Message-State: AOAM532CjpFUrqKWWC49qGxkYLFSC+alHjMVTlaVPhdN+febhfr9H3HU 5nHyVtwKZXkNd1yeyOAXrcL2Glq4R0QW+g== X-Google-Smtp-Source: ABdhPJzSqyGHdLLNaMFHwnojXdJjP/15UrmtdyHoLAz+OLNybb5l4zhUt7CmN5fUo0ELk6+YGGYTmQ== X-Received: by 2002:ae9:c30e:: with SMTP id n14mr11699438qkg.291.1614544262596; Sun, 28 Feb 2021 12:31:02 -0800 (PST) Received: from crass-HP-ZBook-15-G2 (47-218-208-29.bcstcmtk03.res.dyn.suddenlink.net. [47.218.208.29]) by smtp.gmail.com with ESMTPSA id v187sm11361037qkd.50.2021.02.28.12.31.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 28 Feb 2021 12:31:02 -0800 (PST) Date: Sun, 28 Feb 2021 14:31:00 -0600 From: Glenn Washburn To: Daniel Kiper Cc: grub-devel@gnu.org Subject: Re: [PATCH v4 00/13] error: Do compile-time format string checking on grub_error Message-ID: <20210228143100.7e628ea8@crass-HP-ZBook-15-G2> In-Reply-To: <20210227121917.wz7mxdb2znin4pt6@tomti.i.net-space.pl> References: <20210227121917.wz7mxdb2znin4pt6@tomti.i.net-space.pl> Reply-To: development@efficientek.com X-Mailer: Claws Mail 3.17.4 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=2607:f8b0:4864:20::72b; envelope-from=development@efficientek.com; helo=mail-qk1-x72b.google.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 28 Feb 2021 20:31:15 -0000 On Sat, 27 Feb 2021 13:19:17 +0100 Daniel Kiper wrote: > On Thu, Feb 18, 2021 at 08:47:01PM -0600, Glenn Washburn wrote: > > This patch series fixes all compile errors due to format string > > issues on grub_error. This was tested against nearly all supported > > platforms successfully. This is important because earlier versions > > of these changes compiled successfully on x86 platforms, but had > > issues on other ones not tested. > > Could you give examples what kind of errors you get when you build > non-x86 platforms? More format string errors showed up when building for non-x86 platforms because code which contained a format string issue, but is not compiled for x86 was then compiled. For example, the diff for grub-core/kern/arm64/dl.c in patch #10. That bug did not show up in my original patch series because I was only building for x86 targets. > > All patches except the last fix actual format string issues. The > > last patch turns format string issues into errors. This is a good > > idea because it will help to prevent introducing new format string > > issues into the code. Since, I presume, Daniel does not do not do a > > build test for all architectures before committing to master, this > > will not ensure that no format string issues get introduced into > > the code. However, it should flush out any format string > > I am confused by these sentence. Anyway, I do build tests of all > patches before committing for all architectures which I am aware of. > I think difference between our results come from difference in build > environments, flags and options to do tests. I hope this did not sound like a criticism or that you missed something due to the way you are build testing. You would have never gotten a build failure for any of these bugs even if you were building for all architectures (which it sounds like you were). I was merely unsure if you did a build test for all known architectures for every update of master. With patch #13 it can be more of an issue if you're not building testing all architectures because of the situation I outlined above in the first paragraph. In the worst case though, GRUB would fail to build for certain non-tested architectures where patches with format string bugs were introduced. Presumably, this would be discovered quickly by the people here who use those architectures. > > issues when the CI build is done. > > > > Many of these changes are fairly obvious. I tried to use the > > PRI*GRUB_*_T macros as much as I could and to not cast arguments. > > There are some notable exceptions. There is some code that uses the > > same grub_error code in both 32 and 64 bit architectures (riscv), > > so casting was needed to force the larger storage type. The second > > to last patch for zfs is still confounding to me as to why the > > macro PRIuGRUB_UINT64_T was not expanding to llu, and yet the > > compiler was saying the argument was a long long unsigned. > > For all the patches except #12 Reviewed-by: Daniel Kiper > > > Though I will probably do not get #13 until #12 is updated and merged. Yes definitely, #13 requires the issue in #12 be addressed in someway. Glenn