ping: fix alignment errors

[jansucan]

Summary

Fix alignment errors

This fixes -Wcast-align errors when compiled with WARNS=6.

Submitted by: Ján Sučan <[e-mail]>
Sponsored by: Google LLC (Google Summer of Code 2019)

[asomers]

This change breaks the build on powerpc, powerpc64, and sparc64:

/home/asomers/freebsd/base/head/sbin/ping/ping.c: In function 'pr_pack':
/home/asomers/freebsd/base/head/sbin/ping/ping.c:1144: warning: signed and unsigned type in conditional expression

[jansucan]

If the line 1144 is this line

if (icp.icmp_type == icmp_type_rsp) {

then it's interesting because both of the variables are u_char. Could you please provide me more info from the compiler?

[asomers]

That's the whole of the compiler's error output. If you want to reproduce it locally, you should be able to do it with env ARCH=powerpc TARGET_ARCH=powerpc make buildworld. I recommend adding -DNO_CLEAN if you do it more than once. And don't forget the appropriate -j X, or you'll be waiting all day.

[jansucan]

Fix the warning about signed and unsigned type in conditional
expression.

[asomers]

sbin/ping/ping.c

1144 ↗

(On Diff #61031)

Would this be more clearly written MIN((int)sizeof(icp), cc)?
And since cc is initialized from the output of recvmsg, shouldn't it be of type ssize_t?

[jansucan]

sbin/ping/ping.c

1144 ↗

(On Diff #61031)

And since cc is initialized from the output of recvmsg, shouldn't it be of type ssize_t?

That's true. Current use of data types in the code is less appropriate than it should be. I will try to fix this in another diff.

[jansucan]

Use MIN() instead of a custom conditional expression.

[jansucan]

sbin/ping/ping.c

1144 ↗

(On Diff #61031)

After doing some experiments it seems that the least error prone way would be to type keep the type casting in (int)sizeof(icp). Changing data type of the variable for recvmsg's return value is OK. The problem is with cc argument to pr_pack. Changing it from int to ssize_t doesn't solve warning about comparing integers with different signs. Changing it from int to size_t (it's a size of a data buffer) starts avalanche of warnings about different signs. If one is corrected, another two appear. Better time for fixing this would be when the code is much more structured so it's much easier to see whether a variable really needs to hold negative values or not.

[asomers]

sbin/ping/ping.c

1144 ↗

(On Diff #61031)

True, a size cast would be needed if cc were ssize_t. But that's still better than the current situation which involves both a sign cast and a size cast (implicitly, when assigning from recvmsg).

[jansucan]

sbin/ping/ping.c

1144 ↗

(On Diff #61031)

Can I change the data type in this diff, or should I create a separate one?

[asomers]

sbin/ping/ping.c

1144 ↗

(On Diff #61031)

It should go into this diff. I don't see any reason to postpone.

[jansucan]

Change data type of the variable for recvmsg's return value from int to ssize_t.

[asomers]

sbin/ping/ping.c

898 ↗

(On Diff #61089)

I don't think there's any reason to change the type of n, too.

[jansucan]

Data type of variable n for return value of pselect doesn't need to be changed.

[jansucan]

sbin/ping/ping.c

898 ↗

(On Diff #61089)

Fixed.

[asomers]

This revision needs to be rebased since the changes to -n and -H.

[jansucan]

Rebase to the git master branch.