[neomutt-devel] Uniformize structures storing operations

[Damien Riegel]

Hi all,


In mutt, we have different ways to declare and define structures that
contain function pointers. It would be nice if we could uniformize that
so that different parts of the codebase would look more alike.

Let me start with three different examples. For each of them, I pasted
excerpts of the structure definition and an example of declaration.

  /********** header cache operations **********/
  typedef void * (*hcache_open_t)(const char *path);
  typedef void (*hcache_free_t)(void *ctx, void **data);
  
  typedef struct
  {
      hcache_open_t    open;
      hcache_free_t    free;
      /* [...] */
  } hcache_ops_t;
  
  /* instantation */
  #define HCACHE_BACKEND_OPS(_name)             \
    const hcache_ops_t hcache_##_name##_ops = { \
      .open    = hcache_##_name##_open,         \
      .free    = hcache_##_name##_free,         \
    };

  HCACHE_BACKEND_OPS(gdbm)


  /**********   mailbox operations    **********/
  struct mx_ops
  {
    int (*open) (struct _context *);
    int (*close) (struct _context *);
    int (*sync) (struct _context *ctx, int *index_hint);
    /* [...] */
  };

  /* instantation */
  struct mx_ops mx_imap_ops = {
    .open = imap_open_mailbox,
    .close = imap_close_mailbox,
    .sync = NULL,      /* imap syncing is handled by imap_sync_mailbox */
  };


  /**********        crypto           **********/
  typedef struct crypt_module_functions
  {
    /* Common/General functions.  */
    crypt_func_init_t init;
    crypt_func_void_passphrase_t void_passphrase;
    /* [...] */
  }
  
  typedef struct crypt_module_specs
  {
    int identifier;			/* Identifying bit.  */
    crypt_module_functions_t functions;
  } *crypt_module_specs_t;
  
  /* instantation */
  struct crypt_module_specs crypt_mod_smime_gpgme =
    { APPLICATION_SMIME,
      {
        crypt_mod_smime_init,
        crypt_mod_smime_void_passphrase,
      }
    };


Here are the differences between them:
 1. Usage of designated initializers (introduced in C99)
 2. Typedef'ing of function pointers
 3. Macro-foo for declaration


1. Designated initializers

In short, one can assign values to designated members during
initialization:
     struct point { int x, y; };
     struct point p = { .y = yvalue, .x = xvalue };

In the code, we can find structures with designated initializers
(mailbox, hcache), and some others using pre-C99 initialization
technique (like crypto modules). Designated initializers have some
advantages: some members can be omitted (and not only the last ones),
members in the structure definition can be reordered painlessly. And
it's part of C99 :)

1. a) Omitted field members

Side question: do we prefer implicit or explicit member initialization?

"Omitted field members are implicitly initialized the same as objects
that have static storage duration.". [1]

So for instance, the `.sync = NULL` in `mx_imap_ops` is not necessary,
but for some people it improves readability. Personally, and especially
for function pointer, I prefer to omit a member instead of assigning it
NULL or its default value.

2. Typedef'ing function pointers

I agree that it improves readability when a function pointer is passed
as an argument to another function. Example from Stack Overflow [2]:

  extern void (*signal(int, void(*)(int)))(int);
vs.
  typedef void (*SignalHandler)(int signum);
  extern  SignalHandler signal(int signum, SignalHandler handler);

But for `ops` stored in a structure like in the three examples above it
doesn't make much difference, it only adds one level of indirection
when reading the code. Moreover in the coding style discussion, it seems
we generally want to avoid typedefs (except in a few cases). So does it
also apply here?

3. Using macros for declaration

In hcache, a macro is used to declare the different kinds of hcache
backend. It prevents some boilerplate code but it forces function naming
and "obfuscates" some info. So for instance, if I want to check in what
context `hcache_gdbm_open` is used it is harder to track because there
is only one occurrence of this symbol: its definition, but no explicit
affectation nor call. Of course, it's easy to understand what's going on
once you saw the macro, but before that it's more confusing.


The point of this thread is to find the method that suits us the best,
and once we agree on that, change existing codebase to make all these
structures more similar. That way, understanding one part of the
codebase helps understanding other parts.
Let's be altruist and friendly (hehe, that sounds like some management
BS but sometimes it's true)!

[1] https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
[2] http://stackoverflow.com/questions/1591361/understanding-typedefs-for-function-pointers-in-c

Thanks,
-- 
Damien