Re: cleanups with long-term benefits (was Re: [PATCH] schemas: Add vim modeline)

[Paolo Bonzini]

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