Re: Adding ifam_pid to ifa_msghdr

[Roy Marples <roy%marples.name@localhost>]

On Wednesday 31 August 2016 14:37:37 Robert Elz wrote:
> You're changing the size of the struct (and incidentally, not doing it
> in a way that is portable to different alignment strategies - you either
> need to explicitly align the new field, or supply padding fields to force
> it)

As the struct needs to be versioned (I can't believe my blindness in not 
seeing that before) I took the opportunity to re-arrange the fields to exactly 
match rtm_msghdr (well, aside from the field ifam_metric as that doesn't exist 
in rtm_msghdr, but the others do). Hopefully this satisfies your alignment 
requirement, otherwise please explain more fully.

> 
> With a different struct size, at the very least (even if the routing
> socket can deal, which I'm not sure of) the sysctl interface needs to
> be versioned.
> 
> Aside from that, this ...
> 
> +#if !defined(COMPAT_14) && !defined(COMPAT_50)
> +			ifam.ifam_pid = curproc->p_pid;
> +#endif
> 
> is certainly not what you want.   You might just as well write that
> as ...
> 
> +/***
> +			ifam.ifam_pid = curproc->p_pid;
> +***/
> 
> for most purposes, as those COMPAT symbols are usually defined.
> You're not using them as intended - the COMPAT_XX symbols should
> be used to add extra (old) code to remain bin compat with earlier
> kernels, not to remove code from current kernels.

rtsock is a special snowflake.
it's designed to be #inlcuded by rtsock_50.c.
When it's #included like so, the structs are changed to compatible ones - this 
allows the kernel to report RTM_VERSION 3 and 4 as needed by compat.
When it's not in compat for rtsock_50.c, COMPAT_14 and COMPAT_50 are undefined, 
this is forced.

I could have written
#ifndef COMPAT_RTSOCK
to achieve the same effect.

I've done that and refreshed the patch - everything is now fully versioned.
Here's the link again:
http://www.netbsd.org/~roy/ifam_pid.patch

I could commit the sysctl_iflist re-factoring as a separate commit.
Comments welcome.

Roy