Re: [PATCH v6 00/14] error: Do compile-time format string checking on grub>

[Glenn Washburn]

On Fri, 5 Mar 2021 17:27:01 +0100
Daniel Kiper <daniel.kiper@oracle.com> wrote:

> On Thu, Mar 04, 2021 at 06:22:31PM -0600, Glenn Washburn wrote:
> > Daniel, you mentioned wanting a separate patch series which is the
> > real fix for patch #12. I've added it to this patch series, since
> > they go together. I can send the single patch as a separate thread
> > if that still desirable.
> 
> For all Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
> 
> I will take all stuff up to #13. The #14 will be applied after 2.06
> release.

Great!

> Thank you for fixing these issues.
>
> By the way, my I ask you once again to send each patch series as
> separate thread. Now you are attaching all patch sets to one cover
> letter which is confusing. Please stop doing that.

How do you pull patch series from the mailing list? I'm curious if this
is messing with that. Also what email client do you use?

You are correct that I'm attaching all new versions of the patch series
to one cover letter, but each patch series also has its own cover
letter. So I don't consider the cover letter that is the root of the
thread to be the cover letter for the new patch series.

I can't find our prior correspondence. I recall saying that the
patchset series in question had been not done in a less than ideal way
because I had start by replying to the cover letter of the prior
patchset and then switched to replying to a particular patchset cover
letter. This was because with experience I determined that attaching to
the thread of the next version of a patchset to the cover letter of the
first version of the patchset looked much nicer in my browser. I
also recall saying that I'd do this in the future to see if it worked
well doing it properly from the start.

My goal is to keep all versions of a patchset together in a client
with tree view of threads (eg. my mail client claws-mail). This makes
it easy to go back to a prior patchset to look at comments. I also
wanted to meet this goal in an aesthetically pleasing way. The first
attempt which attached a new version of a patchset to its priors cover
letter did not meet this because it created a deep tree for patchsets
with lots of revisions. However, attaching each new patchset to the
cover letter of the first patchset, keeps thread tree to a minimum.  It
also makes it to collapse only certain patchset versions (aside from
the first). Also, since I have threads sorted by thread date, I see
patchsets ordered from most recent to least recent, which it how I like
to order all my emails.

The current case does not strictly adhere to these rules because I'm
taking v4 as the initial patchset, which perhaps may be the source of
some confusion. Other than that I think its worked out nicely.

So I'm curious if this is causing some issue with tooling. And I'm
curious what exactly is causing confusion? In my browser its fairly
obvious that the root of the thread is the cover letter for v4 of the
patch series and that the cover letters of attached patch series are
different, noted by a different version number.

Glenn

> Daniel
> 
> > Changes since v5
> >  * Fix formatting issues with spaces around format string macros
> > and casts
> >  * Add extra patch #14 which is the better fix for #12, but needs
> > more testing
> >
> > Glenn
> >
> > Glenn Washburn (14):
> >   misc: Format string for grub_error should be a literal
> >   error: grub_error missing format string argument
> >   error: grub_error format string add missing format code
> >   dmraid_nvidia: Format string error in grub_error
> >   grub_error: Use format code PRIuGRUB_SIZE for variables of type
> >     grub_size_t
> >   pgp: Format code for grub_error is incorrect
> >   efi: Format string error in grub_error
> >   error: Use PRI* macros to get correct format string code across
> >     architectures
> >   error: Use format code PRIxGRUB_UINT64_T for 64-bit uint argument
> > in grub_error
> >   error: Use format code PRIxGRUB_UINT64_T for 64-bit arg in
> > grub_error error: Use format code PRIuGRUB_UINT64_T for 64-bit
> > typed fileblock in grub_error
> >   error: Use format code llu for 64-bit uint bp->blk_prop in
> > grub_error error: Do compile-time format string checking on
> > grub_error zfs: Use grub_uint64_t instead of 1ULL in BF64_*CODE
> > macros