Bug 211394 - [exp-run] POSIX conformance: make functions return void
Summary: [exp-run] POSIX conformance: make functions return void
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Ports Framework (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Ed Schouten
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-07-26 20:36 UTC by Ed Schouten
Modified: 2016-07-31 08:59 UTC (History)
1 user (show)

See Also:


Attachments
Patch for FreeBSD HEAD, likely for 11, maybe even for 10? (5.49 KB, patch)
2016-07-26 20:36 UTC, Ed Schouten
no flags Details | Diff
Don't forget to patch up pw.... (5.49 KB, patch)
2016-07-26 21:49 UTC, Ed Schouten
no flags Details | Diff
Upload the right file. (6.85 KB, patch)
2016-07-26 21:51 UTC, Ed Schouten
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ed Schouten freebsd_committer freebsd_triage 2016-07-26 20:36:30 UTC
Created attachment 173020 [details]
Patch for FreeBSD HEAD, likely for 11, maybe even for 10?

Hi there,

I'd hereby like to request an exp-run to test some changes to function prototypes in our public header files to make them conform to POSIX.

These functions should return void, but in our case return integer values. I guess it's highly unlikely that this will cause build problems, for the reason that (almost) all other operating systems do this correctly.

The attached patch should apply against HEAD. Very likely against 11 as well. Maybe even 10, but I'm not sure.

Thanks in advance!
Comment 1 Ed Schouten freebsd_committer freebsd_triage 2016-07-26 21:49:52 UTC
Created attachment 173023 [details]
Don't forget to patch up pw....
Comment 2 Ed Schouten freebsd_committer freebsd_triage 2016-07-26 21:51:15 UTC
Created attachment 173024 [details]
Upload the right file.
Comment 3 Antoine Brodin freebsd_committer freebsd_triage 2016-07-29 16:59:14 UTC
On amd64 there is 1 new failure:

+ {"origin"=>"security/super", "pkgname"=>"super-3.30.0_1", "phase"=>"build", "errortype"=>"compiler_error"}

Failure log:

http://package18.nyi.freebsd.org/data/110amd64PR211394-default/2016-07-29_14h26m40s/logs/errors/super-3.30.0_1.log
Comment 4 commit-hook freebsd_committer freebsd_triage 2016-07-29 17:19:37 UTC
A commit references this bug:

Author: ed
Date: Fri Jul 29 17:18:48 UTC 2016
New revision: 303495
URL: https://svnweb.freebsd.org/changeset/base/303495

Log:
  Change the return type of freelocale(3) to void.

  Our version of this function currently returns an integer indicating
  failure or success, whereas POSIX specifies that this function has no
  return value. It returns void. Patch up the header, sources and man page
  to use the right type. While there, use the opportunity to simplify the
  body of this function.

  Theoretically speaking, this change breaks the ABI of this function.
  That said, I have yet to find any code that makes use of freelocale()'s
  return value. I couldn't find any of it in the base system, nor did an
  exp-run reveal any breakage caused by this change.

  PR:		211394 (exp-run)

Changes:
  head/include/xlocale/_locale.h
  head/lib/libc/locale/freelocale.3
  head/lib/libc/locale/xlocale.c
Comment 5 commit-hook freebsd_committer freebsd_triage 2016-07-31 08:05:44 UTC
A commit references this bug:

Author: ed
Date: Sun Jul 31 08:05:15 UTC 2016
New revision: 303580
URL: https://svnweb.freebsd.org/changeset/base/303580

Log:
  Fix up setgrent(3) to have a POSIX-compliant prototype.

  Just like with freelocale(3), I haven't been able to find any piece of
  code that actually makes use of this function's return value, both in
  base and in ports. The reason for this is that FreeBSD seems to be the
  only operating system to have such a prototype. This is why I'm deciding
  to not use symbol versioning for this.

  It does seem that the pw(8) utility depends on the function's typing and
  already had a switch in place to toggle between the FreeBSD and POSIX
  variant of this function. Clean this up by always expecting the POSIX
  variant.

  There is also a single port that has a couple of local declarations of
  setgrent(3) that need to be patched up. This is in the process of being
  fixed.

  PR:		211394 (exp-run)

Changes:
  head/include/grp.h
  head/lib/libc/gen/getgrent.3
  head/lib/libc/gen/getgrent.c
  head/usr.sbin/pw/pw_vpw.c
  head/usr.sbin/pw/pwupd.h
Comment 6 Antoine Brodin freebsd_committer freebsd_triage 2016-07-31 08:18:19 UTC
On i386,  there is 1 new failure, the same as on amd64:

+ {"origin"=>"security/super", "pkgname"=>"super-3.30.0_1", "phase"=>"build", "errortype"=>"compiler_error"}
Comment 7 Ed Schouten freebsd_committer freebsd_triage 2016-07-31 08:20:46 UTC
Nice! The change should be pretty safe then. Just let me know whether you approve this change to the security/super port and I'll fix it:

https://reviews.freebsd.org/D7364
Comment 8 commit-hook freebsd_committer freebsd_triage 2016-07-31 08:27:47 UTC
A commit references this bug:

Author: ed
Date: Sun Jul 31 08:27:00 UTC 2016
New revision: 419346
URL: https://svnweb.freebsd.org/changeset/ports/419346

Log:
  Remove local declarations of setgrent().

  In the nearby future, I'm going to change the prototype of the
  setgrent() function. Prevent this port from breaking by removing the
  redundant local declarations of the setgrent() function it has.

  PR:		211394 (exp-run)
  Reviewed by:	antoine
  Differential Revision:	https://reviews.freebsd.org/D7364

Changes:
  head/security/super/files/patch-checks.c
Comment 9 Ed Schouten freebsd_committer freebsd_triage 2016-07-31 08:59:59 UTC
Looks like we're all done!

I guess I won't be committing the changes to encrypt()/setkey() after all, as further grepping reveals that it may be wiser to remove these functions altogether. We're not required to provide them and the only act as stubs. Will file a separate exp-run for this in the future.

Thanks, Antoine!