Re: Diff: Introductory Clause Comma Crap
Mr. GUENTHER: > As a procedural side-note, I would like to suggest that before going on a > quest to make a change that touches so many files cross OpenBSD's code > base, that it would be wise to send out a diff with just a couple examples > and verify that the particular item of concern and the proposed fix is > agreed upon before spending the time to search and edit many other pages. The quoted suggestion is rather good and shall be followed in the future -- in fact, this approach should have been obvious. KUTGW, Varik "NOT A COMPUTER PROGRAMMER!!!" Valefor -- On Sat, 31 Oct 2020 10:47:42 -0900 Philip Guenther wrote: > On Sat, Oct 31, 2020 at 10:19 AM VARIK VALEFOR wrote: > > > This message contains even more grammatical fixes for the OpenBSD > > manual pages. To ensure that the changes which are proposed in this > > message can be justified, this message primarily fixes only a single > > type of error: the absence of commas after introductory clauses. > > > > As a procedural side-note, I would like to suggest that before going on a > quest to make a change that touches so many files cross OpenBSD's code > base, that it would be wise to send out a diff with just a couple examples > and verify that the particular item of concern and the proposed fix is > agreed upon before spending the time to search and edit many other pages. > > This is true not just for documentation changes but code changes, of > course: doing lots of work before there's "buy-in" is risking your time. > > > Philip Guenther
Re: powerpc ld.lld fix
> Date: Sat, 10 Oct 2020 23:19:19 +0200 (CEST) > From: Mark Kettenis > > On powerpc with the secure-plt ABI we need a .got section, even if the > _GLOBAL_OFFSET_TABLE_ symbol isn't referenced. This is needed because > the first three entries of the GOT are used by the dynamic linker. > > With this fix I can build executables of all flavours (including > -static/-nopie). Turns out that adding these GOT entries when using "ld -r" is a bad idea. Dif below fixes that. ok? Index: gnu/llvm/lld/ELF/SyntheticSections.cpp === RCS file: /cvs/src/gnu/llvm/lld/ELF/SyntheticSections.cpp,v retrieving revision 1.2 diff -u -p -r1.2 SyntheticSections.cpp --- gnu/llvm/lld/ELF/SyntheticSections.cpp 11 Oct 2020 13:10:13 - 1.2 +++ gnu/llvm/lld/ELF/SyntheticSections.cpp 31 Oct 2020 23:37:11 - @@ -604,7 +604,7 @@ GotSection::GotSection() // ElfSym::globalOffsetTable. if (ElfSym::globalOffsetTable && !target->gotBaseSymInGotPlt) numEntries += target->gotHeaderEntriesNum; - else if (config->emachine == EM_PPC) + else if (config->emachine == EM_PPC && !config->relocatable) numEntries += target->gotHeaderEntriesNum; }
Diff: Introductory Clause Comma Crap
Sir or Madam: This message contains even more grammatical fixes for the OpenBSD manual pages. To ensure that the changes which are proposed in this message can be justified, this message primarily fixes only a single type of error: the absence of commas after introductory clauses. Unless otherwise specified, for all changes which are proposed in this message, a change adds a comma after an introductory clause; for all introductory clauses, a comma _must_ follow an introductory clause.* However, these changes should not only be made because the changes fix incorrect things; rather, these changes should be made because many changes greatly increase the clarity of the manual pages; incorrect comma usage can lead to the misinterpretation of things, which is bad... especially in the case of a user manual. Mr. MACINTYRE mentioned that VARIK's previous message was fairly unreadable; as such, some spacing has been added to these diffs, thereby hopefully improving the readability of this message. Any advice regarding the formatting of this sort of message would be welcomed; poorly-formatted, i.e., unreadable, messages waste time, and wasting time sucks. KUTGW, Varik "NOT A COMPUTER PROGRAMMER!!!" Valefor *https://owl.purdue.edu/owl/general_writing/punctuation/commas/commas_after_introductions.html = BEGIN DIFFS = diff --git a/bin/csh/csh.1 b/bin/csh/csh.1 index f984356e846..2e89bcc0c3a 100644 --- a/bin/csh/csh.1 +++ b/bin/csh/csh.1 @@ -1039,7 +1039,7 @@ and If braces .Ql { .Ql } -appear in the command form then the modifiers +appear in the command form, then the modifiers must appear within the braces. The current implementation allows only one .Ql \&: @@ -1282,7 +1282,7 @@ is given to the command as its standard input. The file .Ar name is used as the standard output. -If the file does not exist then it is created; +If the file does not exist, then it is created; if the file exists, it is truncated; its previous contents are lost. .Pp If the variable @@ -1469,7 +1469,7 @@ l symbolic link .Pp The specified name is command and filename expanded and then tested to see if it has the specified relationship to the real user. -If the file does not exist or is inaccessible then all enquiries return +If the file does not exist or is inaccessible, then all enquiries return false, i.e., .Ql 0 . Command executions succeed, returning true, i.e., @@ -1477,7 +1477,7 @@ Command executions succeed, returning true, i.e., if the command exits with status 0, otherwise they fail, returning false, i.e., .Ql 0 . -If more detailed status information is required then the command +If more detailed status information is required, then the command should be executed outside an expression and the variable .Ar status examined. @@ -1510,7 +1510,7 @@ non-seekable inputs.) .Ss Built-in commands Built-in commands are executed within the shell. If a built-in command occurs as any component of a pipeline -except the last then it is executed in a sub-shell. +except the last, then it is executed in a sub-shell. .Pp .Bl -tag -width Ds -compact -offset indent .It Ic alias @@ -1562,7 +1562,7 @@ statement as discussed below. .It Ic chdir Ar name Change the shell's working directory to directory .Ar name . -If no argument is given then change to the home directory of the user. +If no argument is given, then change to the home directory of the user. If .Ar name is not found as a subdirectory of the current directory (and does not begin @@ -1759,11 +1759,11 @@ executed (this is a bug). .It Ic endif If the specified .Ar expr -is true then the commands up to the first +is true, then the commands up to the first .Ic else are executed; otherwise if .Ar expr2 -is true then the commands up to the +is true, then the commands up to the second .Ic else are executed, etc. diff --git a/lib/libagentx/subagentx.3 b/lib/libagentx/subagentx.3 index d283ff198e8..23055f4a94c 100644 --- a/lib/libagentx/subagentx.3 +++ b/lib/libagentx/subagentx.3 @@ -524,8 +524,8 @@ Set the return value to an opaque value. .It Fn subagentx_varbind_counter64 Set the return value to an uint64_t of type counter64. .It Fn subagentx_varbind_notfound -When the request is of type GET return an nosuchinstance error. -When the request is of type GETNEXT or GETBULK return an endofmibview error. +When the request is of type GET, return an nosuchinstance error. +When the request is of type GETNEXT or GETBULK, return an endofmibview error. On endofmibview the next object is queried. This function can only be called on objects that contain one or more *_dynamic indices. diff --git a/lib/libc/crypt/crypt.3 b/lib/libc/crypt/crypt.3 index c8ebf9861d4..721b073e8f7 100644 --- a/lib/libc/crypt/crypt.3 +++ b/lib/libc/crypt/crypt.3 @@ -74,7 +74,7 @@ currently supports a single form. If it begins with a string character .Pq Ql $ -and a number then a different algorithm is used depending on the number. +and a number, then a different algorith
Re: Diff: Introductory Clause Comma Crap
On Sat, Oct 31, 2020 at 03:14:30PM -0400, VARIK VALEFOR wrote: > Sir or Madam: > > This message contains even more grammatical fixes for the OpenBSD > manual pages. To ensure that the changes which are proposed in this > message can be justified, this message primarily fixes only a single > type of error: the absence of commas after introductory clauses. > > Unless otherwise specified, for all changes which are proposed in this > message, a change adds a comma after an introductory clause; for all > introductory clauses, a comma _must_ follow an introductory clause.* > However, these changes should not only be made because the changes fix > incorrect things; rather, these changes should be made because many > changes greatly increase the clarity of the manual pages; incorrect > comma usage can lead to the misinterpretation of things, which is > bad... especially in the case of a user manual. > > Mr. MACINTYRE mentioned that VARIK's previous message was fairly > unreadable; as such, some spacing has been added to these diffs, > thereby hopefully improving the readability of this message. Any > advice regarding the formatting of this sort of message would be > welcomed; poorly-formatted, i.e., unreadable, messages waste time, and > wasting time sucks. > hi. Mr. MACINTYRE... you must mean my dad! thank you for your mail. please read guenther's followup - he makes some very good points. commas are really subjective, so a massive comma diff is always likely to be problematic. sentence clauses do not always need commas. sometimes commas just make the text harder to read. look at your very first change: > KUTGW, > Varik "NOT A COMPUTER PROGRAMMER!!!" Valefor > > *https://owl.purdue.edu/owl/general_writing/punctuation/commas/commas_after_introductions.html > > = BEGIN DIFFS = > diff --git a/bin/csh/csh.1 b/bin/csh/csh.1 > index f984356e846..2e89bcc0c3a 100644 > --- a/bin/csh/csh.1 > +++ b/bin/csh/csh.1 > @@ -1039,7 +1039,7 @@ and > If braces > .Ql { > .Ql } > -appear in the command form then the modifiers > +appear in the command form, then the modifiers in a if/then sentence structure, "then" indicates the second clause. the comma is redundant. "then" performs the role of a comma. but depending on the wording, and the number of clauses, a comma might help to make it more readable. but you can;t just add them everywhere. i think you should follow philip's advice to supply a small diff, check whether such changes made wholesale would be welcome, then proceed. i;d be happy to look over a diff where the clauses are not marked with "then", such as here: > diff --git a/lib/libagentx/subagentx.3 b/lib/libagentx/subagentx.3 > index d283ff198e8..23055f4a94c 100644 > --- a/lib/libagentx/subagentx.3 > +++ b/lib/libagentx/subagentx.3 > @@ -524,8 +524,8 @@ Set the return value to an opaque value. > .It Fn subagentx_varbind_counter64 > Set the return value to an uint64_t of type counter64. > .It Fn subagentx_varbind_notfound > -When the request is of type GET return an nosuchinstance error. > -When the request is of type GETNEXT or GETBULK return an endofmibview error. > +When the request is of type GET, return an nosuchinstance error. > +When the request is of type GETNEXT or GETBULK, return an endofmibview error. > On endofmibview the next object is queried. > This function can only be called on objects that contain one or more > *_dynamic > indices. in this case it is really hard to tell where one clause ends and another starts - the comma improves readability. in addition, "an nosuchinstance" should be "a nosuchinstance". good luck! jmc > must appear within the braces. > The current implementation allows only one > .Ql \&: > @@ -1282,7 +1282,7 @@ is given to the command as its standard input. > The file > .Ar name > is used as the standard output. > -If the file does not exist then it is created; > +If the file does not exist, then it is created; > if the file exists, it is truncated; its previous contents are lost. > .Pp > If the variable > @@ -1469,7 +1469,7 @@ l symbolic link > .Pp > The specified name is command and filename expanded and then tested > to see if it has the specified relationship to the real user. > -If the file does not exist or is inaccessible then all enquiries return > +If the file does not exist or is inaccessible, then all enquiries return > false, i.e., > .Ql 0 . > Command executions succeed, returning true, i.e., > @@ -1477,7 +1477,7 @@ Command executions succeed, returning true, i.e., > if the command exits with status 0, otherwise they fail, returning > false, i.e., > .Ql 0 . > -If more detailed status information is required then the command > +If more detailed status information is required, then the command > should be executed outside an expression and the variable > .Ar status > examined. > @@ -1510,7 +1510,7 @@ non-seekable inputs.) > .Ss Built-in commands > Built-in commands are executed within the shell. > If a built-in co
Re: Diff: Introductory Clause Comma Crap
On Sat, Oct 31, 2020 at 10:19 AM VARIK VALEFOR wrote: > This message contains even more grammatical fixes for the OpenBSD > manual pages. To ensure that the changes which are proposed in this > message can be justified, this message primarily fixes only a single > type of error: the absence of commas after introductory clauses. > As a procedural side-note, I would like to suggest that before going on a quest to make a change that touches so many files cross OpenBSD's code base, that it would be wise to send out a diff with just a couple examples and verify that the particular item of concern and the proposed fix is agreed upon before spending the time to search and edit many other pages. This is true not just for documentation changes but code changes, of course: doing lots of work before there's "buy-in" is risking your time. Philip Guenther
Fix ilogb(3)
As reported on bugs@ our ilogb(3) implementation is somewhat buggy. There are several issues: - The amd64 and i386 assembler versions not only return results that don't match the FP_ILOGB0 and FP_ILOGBNAN defines in but ultimately return results that are incompatible with C99 and C11. - The C versions don't use the FP_ILOGB0 and FP_ILOGBNAN defines. - The ilogbl(3) implementation turns into an infinite loop with 182-bit long doubles in some cases. Here is a diff that fixes those issues by: - Dropping the amd64 and i386 versions. Fixing the corner cases in assembly is hard, and the C implementation should be fast enough for regular floating-point values. - Replaces the broken C implementation of ilogbl(3) with separate 128-bit and 80-bit long double implementations. - Add the regress test from FreeBSD. Technically, this should be a libm major bump since the behaviour changes on amd64 and i386. But given that the corner cases were so blatantly wrong on those platforms, I don't think that's necessary. ok? Index: lib/libm/Makefile === RCS file: /cvs/src/lib/libm/Makefile,v retrieving revision 1.120 diff -u -p -r1.120 Makefile --- lib/libm/Makefile 28 Jun 2020 08:22:57 - 1.120 +++ lib/libm/Makefile 31 Oct 2020 14:52:14 - @@ -23,7 +23,7 @@ ARCH_SRCS = e_acos.S e_asin.S e_atan2.S invtrig.c \ s_atan.S s_atanf.S s_ceil.S s_ceilf.S s_copysign.S s_copysignf.S \ s_cos.S s_cosf.S s_floor.S s_floorf.S \ - s_ilogb.S s_ilogbf.S s_log1p.S s_log1pf.S s_logb.S s_logbf.S \ + s_log1p.S s_log1pf.S s_logb.S s_logbf.S \ s_llrint.S s_llrintf.S s_lrint.S s_lrintf.S s_rint.S s_rintf.S\ s_scalbnf.S s_significand.S s_significandf.S \ s_sin.S s_sinf.S s_tan.S s_tanf.S @@ -36,7 +36,7 @@ ARCH_SRCS = e_acos.S e_asin.S e_atan2.S invtrig.c \ s_atan.S s_atanf.S s_ceil.S s_ceilf.S s_copysign.S s_copysignf.S \ s_cos.S s_cosf.S s_floor.S s_floorf.S \ - s_ilogb.S s_ilogbf.S s_log1p.S s_log1pf.S s_logb.S s_logbf.S \ + s_log1p.S s_log1pf.S s_logb.S s_logbf.S \ s_llrint.S s_llrintf.S s_lrint.S s_lrintf.S \ s_rint.S s_rintf.S s_scalbnf.S s_significand.S \ s_significandf.S s_sin.S s_sinf.S s_tan.S s_tanf.S Index: lib/libm/arch/amd64/s_ilogb.S === RCS file: lib/libm/arch/amd64/s_ilogb.S diff -N lib/libm/arch/amd64/s_ilogb.S --- lib/libm/arch/amd64/s_ilogb.S 3 Jul 2018 22:43:34 - 1.5 +++ /dev/null 1 Jan 1970 00:00:00 - @@ -1,20 +0,0 @@ -/* $OpenBSD: s_ilogb.S,v 1.5 2018/07/03 22:43:34 mortimer Exp $ */ -/* - * Written by J.T. Conklin . - * Public domain. - */ - -#include -#include "abi.h" - -ENTRY(ilogb) - RETGUARD_SETUP(ilogb, r11) - movsd %xmm0,-8(%rsp) - fldl-8(%rsp) - fxtract - fstp%st - fistpl -8(%rsp) - movl-8(%rsp),%eax - RETGUARD_CHECK(ilogb, r11) - ret -END_STD(ilogb) Index: lib/libm/arch/amd64/s_ilogbf.S === RCS file: lib/libm/arch/amd64/s_ilogbf.S diff -N lib/libm/arch/amd64/s_ilogbf.S --- lib/libm/arch/amd64/s_ilogbf.S 3 Jul 2018 22:43:34 - 1.5 +++ /dev/null 1 Jan 1970 00:00:00 - @@ -1,20 +0,0 @@ -/* $OpenBSD: s_ilogbf.S,v 1.5 2018/07/03 22:43:34 mortimer Exp $ */ -/* - * Written by J.T. Conklin . - * Public domain. - */ - -#include -#include "abi.h" - -ENTRY(ilogbf) - RETGUARD_SETUP(ilogbf, r11) - movss %xmm0,-4(%rsp) - flds-4(%rsp) - fxtract - fstp%st - fistpl -4(%rsp) - movl-4(%rsp),%eax - RETGUARD_CHECK(ilogbf, r11) - ret -END_STD(ilogbf) Index: lib/libm/arch/i387/s_ilogb.S === RCS file: lib/libm/arch/i387/s_ilogb.S diff -N lib/libm/arch/i387/s_ilogb.S --- lib/libm/arch/i387/s_ilogb.S12 Sep 2016 19:47:02 - 1.6 +++ /dev/null 1 Jan 1970 00:00:00 - @@ -1,23 +0,0 @@ -/* $OpenBSD: s_ilogb.S,v 1.6 2016/09/12 19:47:02 guenther Exp $ */ -/* - * Written by J.T. Conklin . - * Public domain. - */ - -#include "DEFS.h" - -ENTRY(ilogb) - pushl %ebp - movl%esp,%ebp - subl$4,%esp - - fldl8(%ebp) - fxtract - fstp%st - - fistpl -4(%ebp) - movl-4(%ebp),%eax - - leave - ret -END_STD(ilogb) Index: lib/libm/arch/i387/s_ilogbf.S === RCS file: lib/libm/arch/i387/s_ilogbf.S diff -N lib/libm/arch/i387/s_ilogbf.S --- lib/libm/arch/i387/s_ilogbf.S 12 Sep 2016 19:47:02 - 1.6 +++ /dev/null 1 Jan 1970 00:00:00 - @@ -1,23 +0,0 @@ -/* $OpenBSD: s_ilogbf.S,v 1.6 2016/09/12 19:47:02 guenther Exp $ */ -/* - * Wri
Re: [PATCH] wireguard: release correct lock on exceptional case
On Sat, Oct 31, 2020 at 03:03:10PM +0100, Jason A. Donenfeld wrote: > Backtrace from Jesper: > > ddb{0}> show panic > noise_keypair: lock not held > ddb{0}> trace > db_enter() at db_enter+0x10 > panic(81db9b58) at panic+0x12a > rw_exit_write(8801ed10) at rw_exit_write+0xb5 > noise_remote_begin_session(8801ec10) at > noise_remote_begin_session+0x3c > 1 > wg_send_response(8801ebe0) at wg_send_response+0x7b > wg_handshake(80588000,fd800e7b5a00) at wg_handshake+0x576 > wg_handshake_worker(80588000) at wg_handshake_worker+0x48 > taskq_thread(80049200) at taskq_thread+0x81 > end trace frame: 0x0, count: -8 > ddb{0}> machine ddbcpu 1 > > Reported-by: Jasper Lievisse Adriaanse > --- > sys/net/wg_noise.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git sys/net/wg_noise.c sys/net/wg_noise.c > index 66bdecee80e..adb00568eb4 100644 > --- sys/net/wg_noise.c > +++ sys/net/wg_noise.c > @@ -459,7 +459,7 @@ noise_remote_begin_session(struct noise_remote *r) > NOISE_SYMMETRIC_KEY_LEN, NOISE_SYMMETRIC_KEY_LEN, 0, 0, > hs->hs_ck); > } else { > - rw_exit_write(&r->r_keypair_lock); > + rw_exit_write(&r->r_handshake_lock); > return EINVAL; > } > > -- > 2.29.1 Thanks for the quick fix, I've just committed this. Cheers, -- jasper
[PATCH] wireguard: release correct lock on exceptional case
Backtrace from Jesper: ddb{0}> show panic noise_keypair: lock not held ddb{0}> trace db_enter() at db_enter+0x10 panic(81db9b58) at panic+0x12a rw_exit_write(8801ed10) at rw_exit_write+0xb5 noise_remote_begin_session(8801ec10) at noise_remote_begin_session+0x3c 1 wg_send_response(8801ebe0) at wg_send_response+0x7b wg_handshake(80588000,fd800e7b5a00) at wg_handshake+0x576 wg_handshake_worker(80588000) at wg_handshake_worker+0x48 taskq_thread(80049200) at taskq_thread+0x81 end trace frame: 0x0, count: -8 ddb{0}> machine ddbcpu 1 Reported-by: Jasper Lievisse Adriaanse --- sys/net/wg_noise.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git sys/net/wg_noise.c sys/net/wg_noise.c index 66bdecee80e..adb00568eb4 100644 --- sys/net/wg_noise.c +++ sys/net/wg_noise.c @@ -459,7 +459,7 @@ noise_remote_begin_session(struct noise_remote *r) NOISE_SYMMETRIC_KEY_LEN, NOISE_SYMMETRIC_KEY_LEN, 0, 0, hs->hs_ck); } else { - rw_exit_write(&r->r_keypair_lock); + rw_exit_write(&r->r_handshake_lock); return EINVAL; } -- 2.29.1