[BTRFS] Code refactoring and integrate with fs_shell

[hyche]

Refactoring btrfs source:

• Fixed style violation
• System header files should be the first to be included

Also if FS_SHELL is defined, compile with fs_shell headers instead.

[owenca]

Replying to hyche:

• System header files should be the first to be included

Except that the CPP file's own header should be the first #include to ensure it's self-contained, which is the case with the headers in your patch.

From ​https://www.haiku-os.org/development/coding-guidelines, "the header that belongs to a source file should be included first to ensure self containment".

[pulkomandy]

• The asterisk style is not fixed (next to the type or next to the variable). As long as it is consistent within one file
• The indentation in class declarations was correct before your changes. The idea is to have one column for modifiers (virtual, const, static, etc), one column for types, and one column for variables and function names.
• There are always two blank lines between blocks (for example AttributeIterator.h line 12, between the includes and the class declaration - also in the other files)

If possible separate the style changes and fs_shell wrpaping in two commits. Also, you don't need to move <new> and <lib.h> if they are the same in both cases.

[hyche]

Replying to pulkomandy:

• The asterisk style is not fixed (next to the type or next to the variable). As long as it is consistent within one file

I used the tool "checkstyle.py", it told me reference/pointer should be next to type.

• The indentation in class declarations was correct before your changes. The idea is to have one column for modifiers (virtual, const, static, etc), one column for types, and one column for variables and function names.

• There are always two blank lines between blocks (for example AttributeIterator.h line 12, between the includes and the class declaration - also in the other files)

If possible separate the style changes and fs_shell wrpaping in two commits.

Will fix in next patches!

Also, you don't need to move <new> and <lib.h> if they are the same in both cases.

I need to move those 2 headers to make sure they are included after fs_shell wrapper.

[pulkomandy]

I need to move those 2 headers to make sure they are included after fs_shell wrapper.

Ok, in that case a comment at the inclusion site may be useful:

// This needs to be included after the fs_shell wrapper
#include <zlib.h>

[hyche]

I add more changes in btrfs_shell, It fixes the crash when run btrfs_shell on a device.

[pulkomandy]

// This needs to be included after the fs_shell wrapper
#include <zlib.h>
#include <new>
    
#include "fssh_api_wrapper.h"

They appear to be included before the wrapper, now?

I'm not sure about the indentation style for inline methods braces:

    void    ThisLooksStrange {
                DoStuff(); }

I would stick with how it was before:

    void    ThisLooksBetter
                { DoStuff(); }

We should adjust the coding guidelines if it is not mentionned there, and the checkstyle tool if it detects it. The tool is not always right.

Maybe someone else can comment on the exact formatting to use in that case?

I think you can remove the libHaikuCompat thing in your jamfile (and in bfs_shell one too, if it's still there). This was used to run the tools on BeOS, but we don't support that anymore.

[hyche]

Replying to pulkomandy:

// This needs to be included after the fs_shell wrapper
#include <zlib.h>
#include <new>
    
#include "fssh_api_wrapper.h"

They appear to be included before the wrapper, now?

After something mean it will be done first, right ? I'm confused now.

I'm not sure about the indentation style for inline methods braces:

    void    ThisLooksStrange {
                DoStuff(); }

I would stick with how it was before:

    void    ThisLooksBetter
                { DoStuff(); }

There are 2 styles like this in the code base, I just chose one and rewrite. As you can see ​here and ​here

I think you can remove the libHaikuCompat thing in your jamfile (and in bfs_shell one too, if it's still there). This was used to run the tools on BeOS, but we don't support that anymore.

Got it!

[pulkomandy]

Applied in hrev51282, sorry for the delay. You can submit more things for upstreaming, if more parts of your code are ready. It's ok to open a ticket and point to specific commits on your github fork, no need to submit them as patches. But the ticket is important as it helps tracking what to merge.