• Mathieu Desnoyers's avatar
    staging: lttng: cleanup one-bit signed bitfields · 0dcbcbb4
    Mathieu Desnoyers authored
    * Dan Carpenter <dan.carpenter@oracle.com> wrote:
    > Sparse complains that these signed bitfields look "dubious".  The
    > problem is that instead of being either 0 or 1 like people would expect,
    > signed one bit variables like this are either 0 or -1.  It doesn't cause
    > a problem in this case but it's ugly so lets fix them.
    
    * walter harms (wharms@bfs.de) wrote:
    > hi,
    > This patch looks ok to me but this design is ugly by itself.
    > It should be replaced by an uchar uint whatever or use a
    > real bool (obviously not preferred by this programmes).
    
    bool :1, uchar :1 or uint :1 could make sense. uchar:1/bool:1 won't save
    any space here, because the surrounding fields are either uint or
    pointers, so alignment will just add padding.
    
    I try to use int/uint whenever possible because x86 CPUs tend to get
    less register false-dependencies when using instructions modifying the
    whole register (generated by using int/uint types) rather than only part
    of it (uchar/char/bool). I only use char/uchar/bool when there is a
    clear wanted space gain.
    
    The reason why I never use the bool type within a structure when I want
    a compact representation is that bool takes a whole byte just to
    represent one bit:
    
    struct usebitfield {
        int a;
        unsigned int f:1, g:1, h:1, i:1, j:1;
        int b;
    };
    
    struct usebool {
        int a;
        bool f, g, h, i, j;
        int b;
    };
    
    struct useboolbf {
        int a;
        bool f:1, g:1, h:1, i:1, j:1;
        int b;
    };
    
    int main()
    {
        printf("bitfield %d bytes, bool %d bytes, boolbitfield %d bytes\n",
                sizeof(struct usebitfield), sizeof(struct usebool),
                sizeof(struct useboolbf));
    }
    
    result:
    
    bitfield 12 bytes, bool 16 bytes, boolbitfield 12 bytes
    
    This is because each bool takes one byte, while the bitfields are put in
    units of "unsigned int" (or bool for the 3rd struct). So in this
    example, we need 5 bytes + 3 bytes alignment for the bool, but only 4
    bytes to hold the "unsigned int" unit for the bitfields.
    
    The choice between bool and bitfields must also take into account the
    frequency of access to the variable, because bitfields require mask
    operations to access the selected bit(s). You will notice that none of
    these bitfields are accessed on the tracing fast-path: only in
    slow-paths. Therefore, space gain is more important than speed here.
    
    One might argue that I have so few of these fields here that it does not
    make an actual difference to go for bitfield or bool. I am just trying
    to choose types best suited for their intended purpose, ensuring they
    are future-proof and will allow simply adding more fields using the same
    type, as needed.
    
    So I guess I'll go for uint :1.
    Reported-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
    Signed-off-by: default avatarMathieu Desnoyers <mathieu.desnoyers@efficios.com>
    Acked-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
    0dcbcbb4
ring_buffer_frontend.c 52.1 KB