In ypbind, eliminate error: dereferencing type-punned pointer will break strict-aliasing rules

[araujo]

Summary

Without this fix, compiling ypbind with gcc 4.9 will give warnings such as this:

/opt2/branches/head/usr.sbin/ypbind/ypbind.c: In function 'rpc_received':
/opt2/branches/head/usr.sbin/ypbind/ypbind.c:948:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
  *(u_int32_t *)&ybr.ypbind_resp_u.ypbind_bindinfo.ypbind_binding_addr = raddrp->sin_addr.s_addr;
  ^

[araujo]

Hi Craig,

Could you take a look on this patch too? https://people.freebsd.org/~araujo/ypbind.patch
I already integrated your changes from this revision.

Maybe we should consider to sync it with the current OpenBSD version, although I'm not familiar with ypbind, maybe it is possible.
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.sbin/ypbind/ypbind.c?annotate=1.63

Best,

[dim]

usr.sbin/ypbind/ypbind.c

951–954 ↗

(On Diff #6875)

Missing an & here?

[• rodrigc]

Sorry for the late response. Please address @dim 's comment, if you have not
addressed it already.

Also, removing the type-alias punned warnings is trickier than the other types
of fixes that we have done to clean up the code.
For this change, I would feel more comfortable if we actually tested ypbind with this code change.

I know, this is a headcahe. :(
But still, we have to do the right thing.

Regarding importing ypbind from OpenBSD. I think this is a good idea, because it looks like they more actively maintain the code.
However, this code in FreeBSD is *old*....almost 17 years old. Even if we import from OpenBSD, I don't know if
there are changes to functionality, command-line options, etc. that will be introduced.
This will require testing also.

It depends on how much interest you have in ypbind. :)
I don't have a lot of interest, and was just looking at doing minimal code changes to fix compiler warnings.

Let me know what approach you want to take. I will support you.

[araujo]

In D2690#54413, @rodrigc wrote:

Sorry for the late response. Please address @dim 's comment, if you have not
addressed it already.

Yes, I have addressed @dim's comment already.

Also, removing the type-alias punned warnings is trickier than the other types
of fixes that we have done to clean up the code.
For this change, I would feel more comfortable if we actually tested ypbind with this code change.

I know, this is a headcahe. :(
But still, we have to do the right thing.

I would prefer make it in two steps, first fix the CLANG issues with this patch, I will make the tests and report to you as soon as I get the results. You are right and we must do it.

Regarding importing ypbind from OpenBSD. I think this is a good idea, because it looks like they more actively maintain the code.
However, this code in FreeBSD is *old*....almost 17 years old. Even if we import from OpenBSD, I don't know if
there are changes to functionality, command-line options, etc. that will be introduced.
This will require testing also.

It depends on how much interest you have in ypbind. :)
I don't have a lot of interest, and was just looking at doing minimal code changes to fix compiler warnings.

Let me know what approach you want to take. I will support you.

I don't have too many interest on ypbind, but I do believe to sync it with OpenBSD won't be that hard, but will cost time. I will do it in a second time, first I will check what are the changes and the impact as we are using a very old ypbind.

Thanks for the support, very appreciated, I will start it soon.

[araujo]

I will take it to be able to update the patch that I'm gonna start to test as dicussed with @rodrigc.

[araujo]

Update @rodrigc's patch with dim's review and with my changes too.
I'm gonna start to test it today!

[dim]

usr.sbin/ypbind/ypbind.c

621–622 ↗

(On Diff #6875)

While we are here, can we please update this to a post-K&R definition? E.g.:

static bool_t broadcast_result(caddr_t out, struct sockaddr_in *addr)

[dim]

For the rest, LGTM.

[araujo]

Address dim@ comment related with the function: broadcast_result().
Also I setup my machine following this page: https://www.freebsd.org/doc/handbook/network-nis.html

Seems everything works fine.

Result:
root@srcCODE:~ # ypcat passwd
smmsp:*:25:25:Sendmail Submission User:/var/spool/clientmqueue:/usr/sbin/nologin
hast:*:845:845:HAST unprivileged user:/var/empty:/usr/sbin/nologin
bin:*:3:7:Binaries Commands and Source:/:/usr/sbin/nologin
tty:*:4:65533:Tty Sandbox:/:/usr/sbin/nologin
kmem:*:5:65533:KMem Sandbox:/:/usr/sbin/nologin
sshd:*:22:22:Secure Shell Daemon:/var/empty:/usr/sbin/nologin
proxy:*:62:62:Packet Filter pseudo-user:/nonexistent:/usr/sbin/nologin
unbound:*:59:59:Unbound DNS Resolver:/var/unbound:/usr/sbin/nologin
_pflogd:*:64:64:pflogd privsep user:/var/empty:/usr/sbin/nologin
nobody:*:65534:65534:Unprivileged user:/nonexistent:/usr/sbin/nologin
root:*:0:0:Charlie &:/root:/bin/csh
toor:*:0:0:Bourne-again Superuser:/root:
daemon:*:1:1:Owner of many system processes:/root:/usr/sbin/nologin
operator:*:2:5:System &:/:/usr/sbin/nologin
man:*:9:9:Mister Man Pages:/usr/share/man:/usr/sbin/nologin
games:*:7:13:Games pseudo-user:/usr/games:/usr/sbin/nologin
news:*:8:8:News Subsystem:/:/usr/sbin/nologin
bind:*:53:53:Bind Sandbox:/:/usr/sbin/nologin
uucp:*:66:66:UUCP pseudo-user:/var/spool/uucppublic:/usr/local/libexec/uucp/uucico
www:*:80:80:World Wide Web Owner:/nonexistent:/usr/sbin/nologin
mailnull:*:26:26:Sendmail Default User:/var/spool/mqueue:/usr/sbin/nologin
_dhcp:*:65:65:dhcp programs:/var/empty:/usr/sbin/nologin
pop:*:68:6:Post Office Owner:/nonexistent:/usr/sbin/nologin
auditdistd:*:78:77:Auditdistd unprivileged user:/var/empty:/usr/sbin/nologin
git_daemon:*:964:964:git daemon:/nonexistent:/usr/sbin/nologin

[araujo]

Minor change to respect style(9).