On 31/07/20 19:05, Daniel P. Berrangé wrote:
> NB our files are not JSON documents, they are a concatenation of a list
> of JSON documents.
This is not something that editors generally have problems with.
> If you use javascript mode, then emacs will highlight all the javascript
> language keywords in the comments because it doesn't recognise "#" as a
> comment - you need // or /* .. */ for comments in JavaScript
Oops, good point. So yeah, using // comments and double quotes would
fix editor problems and be much more kind to downstreams.
> I'd also suggest replacing fprintf/printf/sprintf with the g*printf*
> equivalents. The benefit here is that GLib guarantees that its variants
> work the same way on Windows as on Linux. They pulled in the GNULIB
> printf impl to replace Microsoft's broken impl.
That should not be an issue to replace. Most of our printf uses anyway
are wrapped within error_report, or they are g_strdup_printf which we're
already preferring to the messy libc asprintf for obvious reasons.
> There's various silly little things like ARRAY_SIZE vs G_N_ELEMENTS,
> and __attribute__ macros - QEMU_NORETURN vs G_GNUC_NORETURN.
> QEMU_BUILD_BUG_ON vs G_STATIC_ASSERT
> There's util/buffer.{c.h} that can be replaced by GString or GArray.
Some of these are no brainers too. QEMU_BUILD_BUG_ON reverses the
logic; I'm not sure if Coccinelle can do De Morgan laws.
>>> Another example would be adopting a standard code style and using a
>>> tool like clang-format to enforce this for entire of existing code
>>> base and future contributions and throwing away our checkpatch.pl
>>> which nearly everyone is scared of touching as it is Perl code.
>> Checkpatch does much more than that, though the scary part is indeed the
>> one that enfoces coding style. I wouldn't have a problem with using
>> clang-format and calling it a day.
> If there are things missing that we consider important, a long term
> better strategy would be to use the Python binding to libclang to
> detect the problem, instead of trying to parse C code with Perl and
> regexes.
Most of it is simply "use this function instead of this one" or "place a
comment to explain why you're using this". The main feature of
checkpatch.pl however is that it works on patches, not just files, but
still there would be a substantial advantage in employing clang-format.
Paolo
