svn commit: r358655 - head/sbin/mount_nfs

[Dimitry Andric]

On 5 Mar 2020, at 22:01, Gleb Smirnoff <glebius at FreeBSD.org> wrote:
> 
> On Thu, Mar 05, 2020 at 09:30:41PM +0300, Slawa Olhovchenkov wrote:
> S> > > On Thu, Mar 05, 2020 at 08:35:15PM +0300, Slawa Olhovchenkov wrote:
> S> > > S> > > D> sbin/mount_nfs/mount_nfs.c:549:10: error: cast from 'char *' to 'struct
> S> > > S> > > D> if_msghdr *' increases required alignment from 1 to 4
> S> > > S> > > D> [-Werror,-Wcast-align]
> S> > > S> > > D>                         ifm = (struct if_msghdr *)buf;
> S> > > S> > > D>                               ^~~~~~~~~~~~~~~~~~~~~~~
> S> > > S> > > D> 1 error generated.
> S> > > S> > > D>
> S> > > S> > > D> In practice I don't think the buffer can ever get misaligned, so can you
> S> > > S> > > D> please add a NO_WCAST_ALIGN= to the Makefile?
> S> > > S> > >
> S> > > S> > > route(8) handles the same problem via intermediate (void *) cast. What is
> S> > > S> > > preferred way to solve the problem? Change compiler flags file wide, or
> S> > > S> > > just through (void *) cast?
> S> > > S> >
> S> > > S> > Copy to aligned buffer or got SIGBUS on some architectures?
> S> > > S>
> S> > > S> char buf[2048] __aligned(__alignof(struct if_msghdr));
> S> > > S>
> S> > > S> resolve this watning.
> S> > >
> S> > > Thanks, Slawa! I think this is the most elegant solution.
> S> >
> S> > Why don't just declare the buffer as:
> S> >
> S> >   struct if_msghdr buf;
> S> >
> S> > and then do:
> S> >
> S> >   nread = read(s, &buf, sizeof buf);
> S> >
> S> > ?  You are never reading more than one if_msghdr anyway, and then there
> S> > is no need to cast anything.
> S>
> S> My inspiration: route socket can return other messages (man 4 route)
> 
> Yes, exactly. We don't know what size next datagram is going to be.

Oh, in that case this code seems completely wrong.  How do you know the
full datagram will be delivered with one read() call?

If it always is, then there is no need to read more than the size of
struct if_msghdr, since you are not using any data beyond it.  So in
that case, you can suffice with read(..., sizeof(if_msghdr)).

If the read() call will return partial results, you must repeatedly call
it in a loop, until you either reach EOF, or have enough data. In that
case, a buffer with the size of if_msghdr is also enough, since you
never need to read beyond that.

-Dimitry

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 223 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20200305/ef4e7c3b/attachment.sig>