(+ A few people)
On 01/12/2022 09:21, Edwin Torok wrote:
On 1 Dec 2022, at 09:11, Julien Grall <jul...@xen.org> wrote:
Hi Edwin,
The title should have "OCaml" to clarify that .clang-format is not added at the
root level.
Sure, I'll update that when I resend.
On 30/11/2022 17:32, Edwin Török wrote:
Add a .clang-format configuration that tries to match CODING_STYLE where
possible.
I was not able to express the special casing of braces after 'do'
though, this can only be controlled generally for all control
statements.
It is imperfect, but should be better than the existing bindings, which
do not follow Xen coding style.
Right, from previous discussion, I was under the impression that it requires
some work to write a clang-format file for Xen.
I am hopeful that some day we will have a proper one. In fact, we have been
discussing about this as part of MISRA (+ Stefano).
Add this to tools/ocaml first because:
* there are relatively few C files here, and it is a good place to start with
* it'd be useful to make these follow Xen's CODING_STYLE
(which they currently do not because they use tabs for example)
* they change relatively infrequently, so shouldn't cause issues with
backporting security fixes (could either backport the reindentation
patch too, or use git cherry-pick with `-Xignore-space-change`)
Once this is used it'll need inserting some '#include <stdint.h>', otherwise
xs_wire.h
fails to compile due to the missing uint32_t define.
At first I was a bit concerned with this paragraph because a coding style
should not impact compilation. But I guess that's because the format will
convert u32 to uint32_t. Is that correct?
If so, I would expand the paragraph to explicit say that, per the coding styel,
u32 will be converted to uint32_t.
clang-format rearranges the order of '#include' in C files, it shouldn't
convert types.
But rearranging (sorting) includes can sometimes reveal bugs where the code
only happened to compile because the includes were done in a certain order
(e.g. we included something that included stdint.h, therefore the next include
line worked, but if you swap the include order that no longer works), i.e.:
#include "c.h"
#include "b.h"
vs
/* post formatting */
#include "b.h"
#include "c.h"
Where c.h includes a.h, and b.h depends on declarations from a.h, then prior to
reformat the code compiles, and afterwards it doesn't.
Thanks for the detailed information, I think some of it needs to be
summarized in the commmit message.
Which can be fixed by adding this to the C file (and then regardless of include
order of the other 2 it compiles):
#include <a.h>
This would not work if the file were called "d.h" rather than "a.h"
because the clang format would re-order it. So...
Or by fixing b.h to include a.h itself it it depends on it.
... this is the proper way to fix it.
Perhaps this'd better be fixed in xs_wire.h itself to include all its
dependencies, but that is a publicly installed header, and there might be a
reason why it doesn't include stdint.h.
I am not aware of any restrictions and at least one public headers
already include <stdint.h>. I am CCed a few more people to get an opinion.
Cheers,
--
Julien Grall
