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
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
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 $ */ -/* - *
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_keypair_lock);
> + rw_exit_write(>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_keypair_lock);
+ rw_exit_write(>r_handshake_lock);
return EINVAL;
}
--
2.29.1
