| Message ID | 20191007084339.GA7808@dcvr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,01/20] diff: use hashmap_entry_init on moved_entry.ent | expand |
Eric Wong <e@80x24.org> writes: > "struct hashmap_entry" inside "struct subprocess_entry" > no longer needs to be the first member of any struct, > so the old comment is no longer true. Hmm, is that true? struct cmd2process { struct subprocess_entry subprocess; unsigned int supported_capabilities; }; static int start_multi_file_filter_fn(struct subprocess_entry *subprocess) { static int versions[] = {2, 0}; static struct subprocess_capability capabilities[] = { { "clean", CAP_CLEAN }, { "smudge", CAP_SMUDGE }, { "delay", CAP_DELAY }, { NULL, 0 } }; struct cmd2process *entry = (struct cmd2process *)subprocess; return subprocess_handshake(subprocess, "git-filter", versions, NULL, capabilities, &entry->supported_capabilities); } The cast "struct subprocess_entry *" to "struct cmd2process *" we see here does require that the address of the subprocess field must be the same as the address of the structure itself. So I'd have to say that the comment still is true, but not for the reasons of what is in "struct subprocess_entry".
Junio C Hamano <gitster@pobox.com> wrote: > Eric Wong <e@80x24.org> writes: > > > "struct hashmap_entry" inside "struct subprocess_entry" > > no longer needs to be the first member of any struct, > > so the old comment is no longer true. > > Hmm, is that true? > > struct cmd2process { > struct subprocess_entry subprocess; > unsigned int supported_capabilities; > }; > > static int start_multi_file_filter_fn(struct subprocess_entry *subprocess) > { > static int versions[] = {2, 0}; > static struct subprocess_capability capabilities[] = { > { "clean", CAP_CLEAN }, > { "smudge", CAP_SMUDGE }, > { "delay", CAP_DELAY }, > { NULL, 0 } > }; > struct cmd2process *entry = (struct cmd2process *)subprocess; > return subprocess_handshake(subprocess, "git-filter", versions, NULL, > capabilities, > &entry->supported_capabilities); > } > > The cast "struct subprocess_entry *" to "struct cmd2process *" we > see here does require that the address of the subprocess field must > be the same as the address of the structure itself. So I'd have to > say that the comment still is true, but not for the reasons of what > is in "struct subprocess_entry". Oops, right. Let's just leave it at 20 patches for this series and skip this one, for now. I seem to recall there's a bunch of places where we do casts like that which could be rewritten more flexibly with container_of.
diff --git a/convert.c b/convert.c index 94ff837649..93c1e1eae9 100644 --- a/convert.c +++ b/convert.c @@ -753,7 +753,7 @@ static int apply_single_file_filter(const char *path, const char *src, size_t le #define CAP_DELAY (1u<<2) struct cmd2process { - struct subprocess_entry subprocess; /* must be the first member! */ + struct subprocess_entry subprocess; unsigned int supported_capabilities; };