Re: svn commit: r357833 - head/tests/sys/pjdfstest/tests

2020-02-17 Thread Enji Cooper (yaneurabeya) 

> On Feb 12, 2020, at 14:01, Ed Maste  wrote:
> 
> On Wed, 12 Feb 2020 at 15:34, Enji Cooper  > wrote:
>> 
>> On Wed, Feb 12, 2020 at 9:37 AM Ed Maste  wrote:
>>> 
>>> Author: emaste
>>> Date: Wed Feb 12 17:37:32 2020
>>> New Revision: 357833
>>> URL: https://svnweb.freebsd.org/changeset/base/357833
>>> 
>>> Log:
>>>  Tag pjdfstest symlink with pkgbase package
>>> 
>>>  As with the rest of pjdfstest, tag the symlink with package=tests.
>>>  The tests -> . symlink seems a little strange but that's independent
>>>  of pkgbase.
>> 
>> Sidenote: the reason for the symlink was to facilitate integrating the
>> test suite without having to completely refactor the code.
> 
> Thanks for the note - this probably ought to be a comment by the
> symlink; I'll add it at some point if you don't get to it first.

I’ll do that in a bit. I need to dust off the cobwebs with my src bit anyhow, 
lest the reaper reap it :).
-Enji
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r357967 - head/sbin/ping6

2020-02-15 Thread Enji Cooper (yaneurabeya) 


> On Feb 15, 2020, at 07:39, Alexander V. Chernikov  
> wrote:
> 
> Author: melifaro
> Date: Sat Feb 15 15:39:53 2020
> New Revision: 357967
> URL: https://svnweb.freebsd.org/changeset/base/357967
> 
> Log:
>  Make ping6(1) return code consistent with the man page.
>   When every sendto() call originated by ping6(1) fails, current code always
>   returns 2 ("transmission was successful but no responses were received")
>   which is incorrect. Return EX_OSERR instead as in many cases it indicates
>   some kernel-level problems.
> 
>  MFC after:   3 weeks

Could you please add this to RELNOTES?
Thanks!
-Enji
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r357679 - head/sys/kern

2020-02-09 Thread Enji Cooper (yaneurabeya) 


> On Feb 8, 2020, at 07:51, Mateusz Guzik  wrote:
> 
> Author: mjg
> Date: Sat Feb  8 15:51:08 2020
> New Revision: 357679
> URL: https://svnweb.freebsd.org/changeset/base/357679
> 
> Log:
>  vfs: remove now useless ENODEV handling from vn_fullpath consumers
> 
>  Noted by:ngie

Thank you Mateusz :)!
-Enji
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r353305 - head/tests/sys/kern

2019-10-09 Thread Enji Cooper (yaneurabeya) 

> On Oct 8, 2019, at 08:04, Konstantin Belousov  wrote:
> 
> On Tue, Oct 08, 2019 at 01:43:05PM +, Eric van Gyzen wrote:
>> Author: vangyzen
>> Date: Tue Oct  8 13:43:05 2019
>> New Revision: 353305
>> URL: https://svnweb.freebsd.org/changeset/base/353305
>> 
>> Log:
>>  Fix problems in the kern_maxfiles__increase test
>> 
>>  ATF functions such as ATF_REQUIRE do not work correctly in child processes.
>>  Use plain C functions to report errors instead.
> There are much more tests that fork and use ATF_ in children.
> Look e.g. at most ptrace(2) tests.

I beg to disagree:

  86 /*
  87  * A variant of ATF_REQUIRE that is suitable for use in child
  88  * processes.  This only works if the parent process is tripped up by
  89  * the early exit and fails some requirement itself.
  90  */
  91 #define CHILD_REQUIRE(exp) do { \
  92 if (!(exp)) \
  93 child_fail_require(__FILE__, __LINE__,  \
  94 #exp " not met");   \
  95 } while (0)

The issue, in this particular case, and the item that evangyzen was fixing, was 
the fact that failures in children can result in very confusing failures from a 
parent level. In particular, ATF_CHECK does not trickle up to parents and 
ATF_REQUIRE* gets thrown up to parents as abort()’ed processes.

The best way to handle this within the atf-c/atf-c++ framework (with less 
boilerplate) is to use these APIs: atf_utils_fork(..)/atf_utils_wait(..). You 
will still need to use `_exit` (instead of 
exit(..)/assert(..)/ATF_CHECK(..)/ATF_REQUIRE(..), but it’s a lot less 
boilerplate than writing it yourself.

Again, this is why I was driving towards GoogleTest. Despite the fact that it’s 
a C++ framework, there’s a lot less confusing boilerplate involved and context, 
since things are executed in relatively the same context, i.e., setup -> test 
-> teardown, and they’re easier to follow.

The best way to move forward usability-wise with this (I think) is to probably 
alias the ATF_* macros to something more sensible when forking processes. 
However, given the amount of complaints I’ve heard about ATF, I think it’s best 
not to build upon an unstable foundation, but instead encourage the use of 
something more widely-accepted across the open source community/straightforward 
use wise. In this case, googletest.

Thanks,
-Enji
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r346571 - in head: share/examples/tests/tests/tap share/man/man4 share/man/man5 share/zoneinfo/tests usr.bin/calendar/calendars usr.bin/du/tests usr.bin/getconf/tests usr.bin/procstat/

2019-09-03 Thread Enji Cooper (yaneurabeya) 

> On Apr 22, 2019, at 12:19, Edward Napierala  wrote:
> 
> On Mon, 22 Apr 2019 at 20:01, Rodney W. Grimes
>  wrote:
>> 
>>> Author: ngie
>>> Date: Mon Apr 22 17:52:46 2019
>>> New Revision: 346571
>>> URL: https://svnweb.freebsd.org/changeset/base/346571
>>> 
>>> Log:
>>>  Update the spelling of my name
>>> 
>>>  Previous spellings of my name (NGie, Ngie) weren't my legal spelling. Use 
>>> Enji
>>>  instead for clarity.
>>> 
>>>  While here, remove "All Rights Reserved" from copyrights I "own".
> 
> [..]
> 
>>> Modified: head/share/man/man4/cfiscsi.4
>>> ==
>>> --- head/share/man/man4/cfiscsi.4 Mon Apr 22 17:48:10 2019
>>> (r346570)
>>> +++ head/share/man/man4/cfiscsi.4 Mon Apr 22 17:52:46 2019
>>> (r346571)
>>> @@ -1,7 +1,6 @@
>>> .\" Copyright (c) 2013 Edward Tomasz Napierala
>>> .\" Copyright (c) 2015-2017 Alexander Motin 
>>> -.\" Copyright (c) 2017 Ngie Cooper 
>>> -.\" All rights reserved.
>>> +.\" Copyright (c) 2017 Enji Cooper 
>> 
>> We should investiage the history of this All rights reserved,
>> I suspect it actually originally belongs to traz and possibly
>> mav, and not explicity you due to prior habbits.  If you have
>> already done that investigation (ie, you added the line) ignore
>> my comment.  If however you simply added a copyright between
>> the line and some other copyright please restore this line
>> to its prior state.
> 
> FWIW, I'm perfectly fine with this.

The lineage is complicated: trasz@ was the original person that put 
“All rights reserved” in the iscsi.4 copyright, and I based manpage on the 
iscsi.4 manpage, so I thought it was appropriate for me to remove the 
copyright. trasz@ signing off on this resolves the only potential gray area 
with this change.
Thank you though for following up.
-Enji

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r346150 - head/sys/vm

2019-09-03 Thread Enji Cooper (yaneurabeya) 

> On Apr 12, 2019, at 13:10, Tycho Nightingale  wrote:
> 
>> On Apr 12, 2019, at 12:43 PM, Enji Cooper  wrote:
>> 
>> 
>>> On Apr 12, 2019, at 5:46 AM, Tycho Nightingale  wrote:
>>> 
>>> Author: tychon
>>> Date: Fri Apr 12 12:46:25 2019
>>> New Revision: 346150
>>> URL: https://svnweb.freebsd.org/changeset/base/346150
>>> 
>>> Log:
>>> for a cache-only zone the destructor tries to destroy a non-existent keg
>>> 
>>> Reviewed by:markj
>>> Sponsored by:   Dell EMC Isilon
>>> Differential Revision:  https://reviews.freebsd.org/D19835
>> 
>> 
>> Hi Tycho!
>>  Is the goal of this change to prevent a panic (with an INVARIANTS 
>> kernel)?
>> Thank you!
>> -Enji
> 
> Hi,
> 
> The goal of this change is to fix a regression introduced in r343026.  It 
> does cause a panic independent of INVARIANTS.  Fortunately there isn’t 
> currently any in-tree code which triggers the panic.  It does however show up 
> when running with the patch under review in D19845.

Hi again!
Thank you for the explanation! This part was missing from the commit 
message, hence my question. I wanted to make sure I understood the change in 
this commit properly.
Cheers!
-Enji

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r350993 - head/sbin/ping6

2019-08-13 Thread Enji Cooper (yaneurabeya) 


> On Aug 13, 2019, at 09:22, Alan Somers  wrote:
> 
> Author: asomers
> Date: Tue Aug 13 16:22:43 2019
> New Revision: 350993
> URL: https://svnweb.freebsd.org/changeset/base/350993
> 
> Log:
>  Consistently use the byteorder functions in the correct direction
> 
>  Though ntohs and htons are functionally identical, they have different 
> meanings.Using the correct one helps to document the code.

This statement is only true for BE platforms. For LE platforms like i386/x64, 
ntohs and htons actually does a endianness conversion:

sys/powerpc/include/endian.h:#define__ntohs(x)  
(__bswap16((__uint16_t)(x)))
sys/powerpc/include/endian.h:#define__ntohs(x)  ((__uint16_t)(x))
sys/sparc64/include/endian.h:#define__ntohs(x)  ((__uint16_t)(x))
sys/x86/include/endian.h:#define__ntohs(x)  __bswap16(x)
sys/mips/include/endian.h:#define   __ntohs(x)  ((__uint16_t)(x))
sys/mips/include/endian.h:#define __ntohs(x)(__bswap16((x)))
sys/arm/include/endian.h:#define __ntohs(x) ((__uint16_t)(x))
sys/arm/include/endian.h:#define __ntohs(x)(__bswap16(x))
sys/arm64/include/endian.h:#define  __ntohs(x)(__bswap16(x))
sys/riscv/include/endian.h:#define  __ntohs(x)(__bswap16(x))
sys/sys/param.h:#define ntohs(x)__ntohs(x)
sys/netinet/in.h:#definentohs(x)__ntohs(x)

Thanks,
-Enji
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r349996 - in head: sbin/ping usr.bin/telnet usr.sbin/inetd

2019-07-15 Thread Enji Cooper (yaneurabeya) 

> On Jul 15, 2019, at 07:19, Warner Losh  wrote:
> 
> Author: imp
> Date: Mon Jul 15 14:19:39 2019
> New Revision: 349996
> URL: https://svnweb.freebsd.org/changeset/base/349996
> 
> Log:
>  Remove all the RELEASE_CRUNCH instances that partially disable IPSEC
> 
>  We remove IPSEC only in parts of the tree, and not others. RELEASE_CRUNCH to
>  disable it has not kept up with all its uses. Remove it. Should there be a 
> real
>  need to disable IPSEC, one that hasn't shown up in the base system to date,
>  it can be re-added behind a WITHOUT_IPSEC build option.

…

> Modified: head/usr.bin/telnet/Makefile
> ==
> --- head/usr.bin/telnet/Makefile  Mon Jul 15 08:39:52 2019
> (r349995)
> +++ head/usr.bin/telnet/Makefile  Mon Jul 15 14:19:39 2019
> (r349996)
> @@ -21,14 +21,8 @@ WARNS?=2
> 
> LIBADD=   telnet ncursesw
> 
> -.if !defined(RELEASE_CRUNCH)
> CFLAGS+=  -DIPSEC
> LIBADD+=  ipsec
> -.else
> -.PATH: ${TELNETDIR}/libtelnet
> -SRCS+=   genget.c getent.c misc.c
> -CFLAGS+= -DHAS_CGETENT
> -.endif

Hmmm… this is interesting. Why was cgetent/ipsec support mutually exclusive…

Thanks for cleaning this up :)!
-Enji
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r349974 - head/libexec/rc/rc.d

2019-07-14 Thread Enji Cooper (yaneurabeya) 

> On Jul 13, 2019, at 09:07, Ian Lepore  wrote:
> 
> Author: ian
> Date: Sat Jul 13 16:07:38 2019
> New Revision: 349974
> URL: https://svnweb.freebsd.org/changeset/base/349974
> 
> Log:
>  Limit access to system accounting files.
> 
>  In 2013 the security chapter of the Handbook was updated in r42501 to
>  suggest limiting access to the system accounting file [*1] by creating the
>  initial file with a mode of 0600. This was in part based on a discussion in
>  the forums [*2]. Unfortunately, this advice is overridden by the fact that a
>  new file is created as part of periodic daily processing, and the file mode
>  is set by the rc.d/accounting script.
> 
>  These changes update the accounting script to create the directory with mode
>  0750 if it doesn't already exist, and to create the daily file with mode
>  0640. This limits write access to root only, read access to root and members
>  of wheel, and eliminates world access completely. For admins who want to
>  prevent even members of wheel from accessing the files, the mode of the
>  /var/account directory can be manually changed to 0700, because the script
>  never creates or changes that directory if it already exists.
> 
>  The accounting_rotate_log() function now also handles the error cases of no
>  existing log file to rotate, and attempting to rotate the file multiple
>  times (.0 file already exists).
> 
>  Another small change here eliminates the complexity of the mktemp/chmod/mv
>  sequence for creating a new acct file by using install(1) with the flags
>  needed to directly create the file with the desired ownership and
>  modes. That allows coalescing two separate if checkyesno accounting_enable
>  blocks into one.
> 
>  These changes were inspired by my investigation of PR 202203.
> 
>  [1] https://www.freebsd.org/doc/handbook/security-accounting.html
>  [2] http://forums.freebsd.org/showthread.php?t=41059
> 
>  PR:  202203
>  Differential Revision:   https://reviews.freebsd.org/D20876

Does this deserve a “Relnotes: yes”…?
Thanks!
-Enji
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r347392 - head/sbin/ifconfig

2019-05-09 Thread Enji Cooper (yaneurabeya) 

> On May 9, 2019, at 05:58, Kyle Evans  wrote:
> 
> Author: kevans
> Date: Thu May  9 12:58:33 2019
> New Revision: 347392
> URL: https://svnweb.freebsd.org/changeset/base/347392
> 
> Log:
>  ifconfig(8): Partial revert of r347241
> 
>  r347241 introduced an ifname <-> kld mapping table, mostly so tun/tap/vmnet
>  can autoload the correct module on use. It also inadvertently made bogus
>  some previously valid uses of sizeof().
> 
>  Revert back to ifkind on the stack for simplicity sake. This reduces the
>  diff from the previous version of ifmaybeload for easiser auditing.

Hi Kyle,
Thank you for this revert. This change fixed the FreeBSD test suite 
runs, which are once again green after this change. A number of tests which use 
ifconfig were failing because they couldn’t configure interfaces: 
https://ci.freebsd.org/job/FreeBSD-head-amd64-test/11134/testReport/ . 
Cheers!
-Enji
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r346571 - in head: share/examples/tests/tests/tap share/man/man4 share/man/man5 share/zoneinfo/tests usr.bin/calendar/calendars usr.bin/du/tests usr.bin/getconf/tests usr.bin/procstat/

2019-04-22 Thread Enji Cooper (yaneurabeya) 

> On Apr 22, 2019, at 12:19, Edward Napierala  wrote:
> 
> On Mon, 22 Apr 2019 at 20:01, Rodney W. Grimes
>  wrote:
>> 
>>> Author: ngie
>>> Date: Mon Apr 22 17:52:46 2019
>>> New Revision: 346571
>>> URL: https://svnweb.freebsd.org/changeset/base/346571
>>> 
>>> Log:
>>>  Update the spelling of my name
>>> 
>>>  Previous spellings of my name (NGie, Ngie) weren't my legal spelling. Use 
>>> Enji
>>>  instead for clarity.
>>> 
>>>  While here, remove "All Rights Reserved" from copyrights I "own".
> 
> [..]
> 
>>> Modified: head/share/man/man4/cfiscsi.4
>>> ==
>>> --- head/share/man/man4/cfiscsi.4 Mon Apr 22 17:48:10 2019
>>> (r346570)
>>> +++ head/share/man/man4/cfiscsi.4 Mon Apr 22 17:52:46 2019
>>> (r346571)
>>> @@ -1,7 +1,6 @@
>>> .\" Copyright (c) 2013 Edward Tomasz Napierala
>>> .\" Copyright (c) 2015-2017 Alexander Motin 
>>> -.\" Copyright (c) 2017 Ngie Cooper 
>>> -.\" All rights reserved.
>>> +.\" Copyright (c) 2017 Enji Cooper 
>> 
>> We should investiage the history of this All rights reserved,
>> I suspect it actually originally belongs to traz and possibly
>> mav, and not explicity you due to prior habbits.  If you have
>> already done that investigation (ie, you added the line) ignore
>> my comment.  If however you simply added a copyright between
>> the line and some other copyright please restore this line
>> to its prior state.
> 
> FWIW, I'm perfectly fine with this.

The lineage is complicated: trasz@ was the original person that put 
“All rights reserved” in the iscsi.4 copyright, and I based manpage on the 
iscsi.4 manpage, so I thought it was appropriate for me to remove the 
copyright. trasz@ signing off on this resolves the only potential gray area 
with this change.
Thank you though for following up.
-Enji
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r346150 - head/sys/vm

2019-04-12 Thread Enji Cooper (yaneurabeya) 

> On Apr 12, 2019, at 13:10, Tycho Nightingale  wrote:
> 
>> On Apr 12, 2019, at 12:43 PM, Enji Cooper  wrote:
>> 
>> 
>>> On Apr 12, 2019, at 5:46 AM, Tycho Nightingale  wrote:
>>> 
>>> Author: tychon
>>> Date: Fri Apr 12 12:46:25 2019
>>> New Revision: 346150
>>> URL: https://svnweb.freebsd.org/changeset/base/346150
>>> 
>>> Log:
>>> for a cache-only zone the destructor tries to destroy a non-existent keg
>>> 
>>> Reviewed by:markj
>>> Sponsored by:   Dell EMC Isilon
>>> Differential Revision:  https://reviews.freebsd.org/D19835
>> 
>> 
>> Hi Tycho!
>>  Is the goal of this change to prevent a panic (with an INVARIANTS 
>> kernel)?
>> Thank you!
>> -Enji
> 
> Hi,
> 
> The goal of this change is to fix a regression introduced in r343026.  It 
> does cause a panic independent of INVARIANTS.  Fortunately there isn’t 
> currently any in-tree code which triggers the panic.  It does however show up 
> when running with the patch under review in D19845.

Hi again!
Thank you for the explanation! This part was missing from the commit 
message, hence my question. I wanted to make sure I understood the change in 
this commit properly.
Cheers!
-Enji
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"