On Tue, 10 May 2022, Julien Grall wrote:
> > It is not reasonable to say "this unrelated thing is broken, and you
> > need to fix it first to get your series in".  Requests like that are,
> > I'm sure, part of what Bertrand raised in the community call as
> > unnecessary fiction getting work submitted.
> 
> To be honest, you put the contributor in this situation. I would have been
> perfectly happy if we keep *cpup* around as there would be only a place to
> fix.
> 
> With this approach, you are effectively going to increase the work later one
> because now we would have to chase all the open-coded version of *cpup* and
> check which one is not safe.


Without disagreeing with Julien or Andrew, I am actually happy to see an
effort to make the review process faster. We have lot of room for
improvement and spotting opportunities to do so is the first step toward
improving the process. I have actually been thinking about how to make
things faster in cases like this and I have a suggesion below.

In this case all of the options are OK. Whether we fix the alignment
problem as part of this patch or soon after it doesn't make much of a
difference. It is more important that we don't get bogged down in a long
discussion. Coding is faster and more fun.

It would take less time for Julien (or Andrew) to write the code than to
explain to the contributor how to do it. English is a good language for
an architectural discussion, but simply replying with the example code
in C would be much faster in cases like this one.

So my suggestion is that it would be best if the reviewer (Julien in
this case) replied directly with the code snipper he wants added. Just
an example without looking too closely:

---
Please do this instead so that alignment also gets fixed:

return be16_to_cpu(*(const __packed uint16_t *)p);
---


Alternatively, I also think that taking this patch as is without
alignment fix (either using be16_to_cpu or be16_to_cpup) is fine. The
alignment could be fixed afterwards. The key is that I think it should
be one of the maintainers to write the follow-up fix. Both of you are
very fast coders and would certainly finish the patch before finishing
the explanation on what needs to be done, which then would need to be
understood and implemented by the contributor (opportunity for
misunderstandings), and verified again by the reviewer on v2. That would
take an order of magnitude more of collective time and effort.

Of course this doesn't apply to all cases, but it should apply to quite
a few.

In short, less English, more C  ;-)