Messages in this thread | | | Subject | Re: [PATCH] HDLC patch for 2.5.5 (0/3) | From | Krzysztof Halasa <> | Date | 19 Feb 2002 12:02:08 +0100 |
| |
Francois Romieu <romieu@cogenit.fr> writes:
> I agree there's a way for an application to cause binary incompatibility if > it does: > > struct userspace_foo { > struct if_settings frob; > int nitz; > } bar; > > If size of struct if_settings changes (increases OR decreases), access to > bar.nitz doesn't work as expected.
I assumed it's union and not a struct, you're right.
> But: > in hdlc_xxx_ioctl, only knowledge of the protocol-related member of the > union > hdlcs_hdlcu is required. Nowhere does the code depend on size of if_settings.
I see now, t seems I haven't read the patches carefully enough.
Now... You just want to introduce an artificial struct which contains only the union... Why? We could use just the union instead (?).
struct hdlc_settings { union { /* sync_serial_settings removed */ raw_hdlc_proto raw_hdlc; cisco_proto cisco; fr_proto fr; fr_proto_pvc fr_pvc; te1_settings te1; } hdlcs_hdlcu; };
Still, te1_settings are interface-related :-) Ok, I assume it goes to the following:
> include/linux/whatever/ioctl.h: > [...] > struct whatever_settings { > union { > /* sync_serial_settings is back */ > sync_serial_settings sync; > fancy_settings fancy; > } > }; > > include/linux/if.h: > [...] > struct if_settings > { > unsigned int type; /* Type of physical device or protocol */ > union { > struct hdlc_settings ifsu_hdlc; > struct whatever_settings ifsu_whatever; > } ifs_ifsu; > }; > > As long as the application only accesses its data and doesn't try to embed > the variable sized kernel structure into its own, it won't break here either.
Yes, the compiler would compile that. Anyway, don't you think it's a little messy? Void * pointers are IMHO not that evil.
Not sure about that, I have to think on it... -- Krzysztof Halasa Network Administrator - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |