Re: About the code style requirement

[Daniel Kiper]

On Fri, Oct 01, 2021 at 03:14:02PM +0800, Qu Wenruo via Grub-devel wrote:
> On 2021/10/1 14:08, Vladimir 'phcoder' Serbinenko wrote:

[...]

> > GRUB already has a btrfs implementation. Writing new one from scratch
> > instead of existing one is unwelcome. From scratch means new bugs.
> > Do you have problems with existing implementation?
>
> In fact, quite some:
>
> - Can't read shared data extents of reflinked files
>   This can be fixed without a big refactor.
>
> - No csum verification
> - No support for extra checksums (xxhash/sha256/blake2)
> - No very basic tree block sanity check
>   The only sanity check is bytenr and fsid, which is far from ideal.
>   The most deadly case like parent-child tree block level mismatch is
>   not handled at all.
>   (which can easily cause a dead loop,
>    e.g. tree_block1 -> tree_block2 -> tree_block 1)
>
> > If so I prefer you
> > to fix the exact bugs rather than attempt to rewrite the entire thing
> > from scratch
>
> If only for the read failure of the reflinked files, I'd be pretty happy
> just to fix that bug.
>
> But for the long run, I really prefer to have a more-or-less common base
> which every btrfs developer can easily start their work easily.
>
> Your work on the existing grub btrfs code is very appreciated, but I'm not
> sure if it is the best solution in the long run.
>
> Thus please let us btrfs developers to contribute our experience to make a
> better and more unified implementation.
> (Although still less unified due to license conflicts)

I think it would be nice to have a kind of library which could be
imported into the GRUB source code in the similar way how it is done
e.g. for gnulib or JSON. Please take a look at grub-core/lib directory
and e.g. commit 528938d50 (json: Import upstream jsmn-1.1.0). Of course
it has to have compatible license with the GRUB. You can find more info
about licensing here [1].

WRT coding style, it is described here [2]. As Daniel A. said it is GNU
coding style with some (minor) exceptions. However, we do not enforce
this coding style for the code which is imported into the GRUB as libraries.

Anyway, we want to have support for latest Btrfs features in the GRUB.
So, I think your proposal is interesting. However, we have to get more
details how to solve various problems. So, if you could consider various
options and present their pros and cons then I think we can continue
discussion how to make Btrfs development for GRUB easier.

Daniel

[1] https://www.gnu.org/licenses/licenses.html.en
[2] https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html