Re: [PATCH 3/4] Introduce "abstract class" to make _locate_element() more independant of diff_rename_dst.

[Jonathan Nieder <jrnieder@gmail.com>]

Yann Dirson wrote:

> --- a/diffcore-rename.c
> +++ b/diffcore-rename.c
> @@ -6,12 +6,16 @@
>  #include "diffcore.h"
>  #include "hash.h"
>  
> +
> +/* "Abstract class" for lists of filespecs */
> +struct dst_obj {
> +	struct diff_filespec *two;
> +};
>  #define locate_element(list,elem,insert_ok)			\
>  	_locate_element(elem, &list##_nr, &list##_alloc,	\
>  			sizeof(*list), insert_ok)
>  
> -/* Table of rename/copy destinations */
> -
> +/* Table of rename/copy destinations, "subclass" of dst_obj */
>  static struct diff_rename_dst {
>  	struct diff_filespec *two;

Nit: language lawyers might be happier with:

  struct dst_obj {
	struct diff_filespec *spec
  };
  struct diff_rename_dst {
	struct dst_obj two;
	...
  };

or

  typedef struct diff_filespec *dst_obj;
  struct diff_rename_dst {
	dst_obj two;
	...
  };

or

  struct diff_rename_dst {
	struct diff_filespec *two;
	...
  };
  ...
  struct diff_filespec **dst = (const char *) rename_dst + next * elem_size;

to avoid violating strict aliasing rules.

Meanwhile, something like

 #define container_of(ptr, type, member) \
	((type *)((char *) ptr - offsetof(type, member)))

can be useful for making the meaning more obvious when downcasting.