[vlc-devel] [PATCH] module_config_t: do not use bitfields

[Rafaël Carré]

Le 25/01/2013 04:37, Rafaël Carré a écrit :

> From c6d57eef37061b7d5495bbbf496537a154177fd0 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Rafa=C3=ABl=20Carr=C3=A9?= <funman at videolan.org>
> Date: Fri, 25 Jan 2013 04:26:06 +0100
> Subject: [PATCH] module_config_t: do not use bitfields
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> The size of union is not guaranteed to fit in 32bits
> This is for example the case when targetting win64
> 
> Reported-by: Mario Speiß <1034-135 at online.de>
                         ^^ thunderbird acting bad it seems
> ---
>  extras/analyser/zsh.cpp                             |    2 +-
>  include/vlc_configuration.h                         |   11 ++++++-----
>  modules/gui/macosx/prefs_widgets.m                  |    2 +-
>  modules/gui/qt4/components/complete_preferences.cpp |    4 ++--
>  modules/gui/qt4/components/preferences_widgets.hpp  |    2 +-

The only users outside of src/ are osx and qt4 interfaces which access:
b_advanced for simple prefs
b_internal (qt4 only) to hide internal options from gui

So we just need 1 or 2 getters for those and we can move the defines to src/

Something like:
bool module_config_getAdvanced(module_config_t*);
bool module_config_getInternal(module_config_t*);
void module_config_getFlags(module_config_t*, bool *advanced, bool
*internal);

This would be nice to have the fix in 2.0.6 win64 I guess.

(zsh hack doesn't count since it's already a über-hack which should only
be built/run immediately after building)

>  src/config/chain.c                                  |    2 +-
>  src/config/cmdline.c                                |    2 +-
>  src/config/core.c                                   |    2 +-
>  src/config/file.c                                   |    6 +++---
>  src/config/help.c                                   |    7 ++++---
>  src/modules/entry.c                                 |   10 +++++-----
>  src/modules/modules.c                               |    4 ++--
>  12 files changed, 28 insertions(+), 26 deletions(-)
> 

> diff --git a/include/vlc_configuration.h b/include/vlc_configuration.h
> index acd1f41..2264f31 100644
> --- a/include/vlc_configuration.h
> +++ b/include/vlc_configuration.h
> @@ -65,11 +65,12 @@ struct module_config_t
>          {
>              uint8_t     i_type;                        /* Configuration type */
>              char        i_short;               /* Optional short option name */
> -            unsigned    b_advanced:1;                     /* Advanced option */
> -            unsigned    b_internal:1;          /* Hidden from prefs and help */
> -            unsigned    b_unsaveable:1;       /* Not stored in configuration */
> -            unsigned    b_safe:1;       /* Safe in web plugins and playlists */
> -            unsigned    b_removed:1;                           /* Deprecated */
> +            uint8_t     bflags;
> +#define MODULE_CONFIG_ADVANCED   (1<<0) /* Advanced option */
> +#define MODULE_CONFIG_INTERNAL   (1<<1) /* Hidden from prefs and help */
> +#define MODULE_CONFIG_UNSAVEABLE (1<<2) /* Not stored in configuration */
> +#define MODULE_CONFIG_SAFE       (1<<3) /* Safe in web plugins and playlists */
> +#define MODULE_CONFIG_REMOVED    (1<<4) /* Deprecated */
>          };
>          uint32_t flags;
>      };

The names clash a bit with VLC_CONFIG_ADVANCED .. although if we put
them in src/ it should be a bit better.