Re: [PATCH] libvirt-domain.h: Fix enum description placement

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jul 21, 2017 at 11:58:45AM +0100, Daniel P. Berrange wrote:
On Fri, Jul 21, 2017 at 12:52:06PM +0200, Michal Privoznik wrote:
There are only two acceptable places for describing enum values.
It's either:

    typedef enum {
        /* Some long description. Therefore it's placed before
         * the value. */
        VIR_ENUM_A_VAL = 1,
    } virEnumA;

or:

    typedef enum {
        VIR_ENUM_B_VAL = 1, /* Some short description */
    } virEnumB;

However, during review of a patch sent upstream I realized that
is not always the case. I went through all the public header
files and identified all the offenders. Luckily there were just
two of them.

Yes, this makes our HTML generated documentation broken, but
that's bug of the generator. Our header files shouldn't be forced
to use something we don't want to.

FWIW, the problem is in the parseEnumBLock() method in apibuild.py

Once it has completed parsing an enum item, it delays adding that
enum to the list until it sees the next item, so that it can capture
the trailing comment.

The only way we can distinguish between a comment that comes before
the enum vs a comment after the enum, on the same line, is by detecting
whitespace (newline) before the trailing comment. Unfortunately I don't
thing this is exposed by the lexer right, so its not entirely easy
to solve.


I was under the impression that this worked, we only broke it by some
recent commit.  I looked at the code and got pretty confused by it, but
shouldn't it be pretty easy (from a big picture view)?  You read until
you have both comment and a member of the struct.

If it's really harder than I think, then we can start using some helper
characters for the comments (at least for now) so that we can properly
identify them, e.g.:

struct meh {
      /*# This is comment for the following member foo */
      unsigned int foo;
      int bar; /*< This is for member bar that's on the same line */
}

and so on.  If that doesn't help either and it never worked,
then... it's a pity :-/


Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
 include/libvirt/libvirt-domain.h | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

Reviewed-by: Daniel P. Berrange <berrange@xxxxxxxxxx>



Regards,
Daniel
--
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux