Bug 217532 - sbin/newfs_nandfs/newfs_nandfs.c warning on unaligned pointer value
Summary: sbin/newfs_nandfs/newfs_nandfs.c warning on unaligned pointer value
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2017-03-04 12:10 UTC by Trond Endrestøl
Modified: 2017-04-13 17:12 UTC (History)
3 users (show)

See Also:


Attachments
Patch for sbin/newfs_nandfs/newfs_nandfs.c eliminating the unaligned pointer value warning (645 bytes, patch)
2017-03-04 12:10 UTC, Trond Endrestøl
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Trond Endrestøl 2017-03-04 12:10:04 UTC
Created attachment 180490 [details]
Patch for sbin/newfs_nandfs/newfs_nandfs.c eliminating the unaligned pointer value warning

sbin/newfs_nandfs/newfs_nandfs.c in 12.0-CURRENT r314649 throws a warning on unaligned pointer value:

newfs_nandfs/newfs_nandfs.c:543:11: error: taking address of packed member 'f_uuid' of class or structure 'nandfs_fsdata' may result in an unaligned pointer value [-Werror,-Waddress-of-packed-member]
        uuidgen(&fsdata.f_uuid, 1);
                 ^~~~~~~~~~~~~
1 error generated.
*** [newfs_nandfs.o] Error code 1

The attached patch fixes this error.
Comment 1 Conrad Meyer freebsd_committer freebsd_triage 2017-03-04 17:00:06 UTC
Thanks!
Comment 2 commit-hook freebsd_committer freebsd_triage 2017-03-04 17:00:10 UTC
A commit references this bug:

Author: cem
Date: Sat Mar  4 16:59:55 UTC 2017
New revision: 314671
URL: https://svnweb.freebsd.org/changeset/base/314671

Log:
  newfs_nandfs: Fix unaligned pointer warning

  PR:		217532
  Submitted by:	Trond Endrestol <Trond.Endrestol at ximalas.info>

Changes:
  head/sbin/newfs_nandfs/newfs_nandfs.c
Comment 3 Dan McGregor 2017-03-06 04:54:55 UTC
This is also very similar to review D9532.
Comment 4 Trond Endrestøl 2017-03-06 08:51:42 UTC
(In reply to Dan McGregor from comment #3)
I was unaware of any the code review on the matter, and independently submitted my own proposal. Feel free to change the name of the variable and replace the assignment with a call to memset() as proposed in the code review.
Comment 5 Dan McGregor 2017-03-06 12:24:58 UTC
I wasn't trying to start anything, just pointing out that something very similar was proposed, reviewed, and accepted already. There is basically one way to solve this, so two people coming up with more or less the same solution independently is expected.

I'm not fussed either way.
Comment 6 Brooks Davis freebsd_committer freebsd_triage 2017-03-06 17:42:25 UTC
It's probably not important in practice, but the memcpy() is probably safer as I don't think that struct assignment is defined in C for misaligned structures.  At the very least, it would be a compiler bug in the memcpy() didn't work, but might be an "optimization" if you took a (potentially unhandled) unhandled alignment fault in the assignment.
Comment 7 Conrad Meyer freebsd_committer freebsd_triage 2017-03-06 17:59:45 UTC
(In reply to Brooks Davis from comment #6)
> I don't think that struct assignment is defined in C for misaligned structures

__packed isn't defined in C, so you don't get misaligned structures at all in standard C.  Still, every compiler that implements __packed must implement misaligned member stores correctly, whether the misaligned member is a struct or scalar.

I think you're imagining a pretty severe compiler bug.
Comment 8 Dan McGregor 2017-03-06 18:53:16 UTC
(In reply to Conrad Meyer from comment #7)

I think you're right. The compiler warning is because a (potentially) misaligned structure pointer is being passed into a function that is expecting an aligned structure pointer. Within this function body a regular assignment should work.
Comment 9 commit-hook freebsd_committer freebsd_triage 2017-04-13 17:12:04 UTC
A commit references this bug:

Author: dim
Date: Thu Apr 13 17:11:51 UTC 2017
New revision: 316772
URL: https://svnweb.freebsd.org/changeset/base/316772

Log:
  MFC r314671 (by cem):

  newfs_nandfs: Fix unaligned pointer warning

  PR:		217532
  Submitted by:	Trond Endrestol <Trond.Endrestol at ximalas.info>

Changes:
_U  stable/10/
  stable/10/sbin/newfs_nandfs/newfs_nandfs.c
_U  stable/11/
  stable/11/sbin/newfs_nandfs/newfs_nandfs.c