uhidppctl(8)

[jwinnie]

Hello OpenBSD developers,

I am interested in contributing to improve the uhidpp(4)
(Logitech Unifying Reciever) support in OpenBSD.

Currently, the uhidpp(4) driver only handles detecting certain
sensors, but I would like more robust support for these devices,
including:

* pairing and unpairing devices on the command-line
* controlling the scrolling speed/auxiliary buttons of wireless mice

Do you know where I should start? Should I work on the uhidpp(4)
driver, or should I go ahead and start writing a command-line utility,
like uhidppctl(8)?

Thank you,
~jwinnie

switch sndiod & aucat to 24-bit ?

[Alexandre Ratchov]

This diff switches internal sample representation frem 16-bit to
24-bit fixed-point. Resampling is already 24-bit only, so there's
little point in keeping the current 16-bit code.

The default device precision doesn't change, i.e. we still request the
same 16-bit mode (and convert everything to 24-bit when needed), so
there's no risk of exposing driver bugs (yet).

If you've the 24-bit capable hardware, adding "-e s24" to sndiod_flags
will reduce the quantization noise, without the need for rebuilding
with -DADATA_BITS=24 anymore.

objections? OK?

Index: sndiod/dsp.h
===
RCS file: /cvs/src/usr.bin/sndiod/dsp.h,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 dsp.h
--- sndiod/dsp.h25 May 2021 08:06:12 -  1.10
+++ sndiod/dsp.h21 Dec 2021 17:53:12 -
@@ -25,28 +25,14 @@
  * boundary is excluded. We represent them as signed fixed point numbers
  * of ADATA_BITS. We also assume that 2^(ADATA_BITS - 1) fits in a int.
  */
-#ifndef ADATA_BITS
-#define ADATA_BITS 16
-#endif
+#define ADATA_BITS 24
 #define ADATA_LE   (BYTE_ORDER == LITTLE_ENDIAN)
 #define ADATA_UNIT (1 << (ADATA_BITS - 1))
 
-#if ADATA_BITS == 16
-
-#define ADATA_MUL(x,y) (((int)(x) * (int)(y)) >> (ADATA_BITS - 1))
-
-typedef short adata_t;
-
-#elif ADATA_BITS == 24
-
 #define ADATA_MUL(x,y) \
((int)(((long long)(x) * (long long)(y)) >> (ADATA_BITS - 1)))
 
 typedef int adata_t;
-
-#else
-#error "only 16-bit and 24-bit precisions are supported"
-#endif
 
 /*
  * The FIR is sampled and stored in a table of fixed-point numbers
Index: sndiod/sndiod.8
===
RCS file: /cvs/src/usr.bin/sndiod/sndiod.8,v
retrieving revision 1.12
diff -u -p -u -p -r1.12 sndiod.8
--- sndiod/sndiod.8 18 Dec 2021 21:41:49 -  1.12
+++ sndiod/sndiod.8 21 Dec 2021 17:53:13 -
@@ -564,11 +564,6 @@ $ sndiod -r 48000 -b 480 -z 240
 Resampling is low quality; down-sampling especially should be avoided
 when recording.
 .Pp
-Processing is done using 16-bit arithmetic,
-thus samples with more than 16 bits are rounded.
-16 bits (i.e. 97dB dynamic) are largely enough for most applications though.
-Processing precision can be increased to 24-bit at compilation time though.
-.Pp
 If
 .Fl a Ar off
 is used,
Index: sndiod/sndiod.c
===
RCS file: /cvs/src/usr.bin/sndiod/sndiod.c,v
retrieving revision 1.47
diff -u -p -u -p -r1.47 sndiod.c
--- sndiod/sndiod.c 1 Nov 2021 14:43:25 -   1.47
+++ sndiod/sndiod.c 21 Dec 2021 17:53:13 -
@@ -85,6 +85,13 @@
 #define DEFAULT_BUFSZ  7680
 #endif
 
+/*
+ * default device precision
+ */
+#ifndef DEFAULT_BITS
+#define DEFAULT_BITS   16
+#endif
+
 void sigint(int);
 void sighup(int);
 void opt_ch(int *, int *);
@@ -486,7 +493,11 @@ main(int argc, char **argv)
pmax = 1;
rmin = 0;
rmax = 1;
-   aparams_init();
+   par.bits = DEFAULT_BITS;
+   par.bps = APARAMS_BPS(par.bits);
+   par.le = ADATA_LE;
+   par.sig = 1;
+   par.msb = 0;
mode = MODE_PLAY | MODE_REC;
dev_first = dev_next = NULL;
port_first = port_next = NULL;
Index: aucat/aucat.1
===
RCS file: /cvs/src/usr.bin/aucat/aucat.1,v
retrieving revision 1.116
diff -u -p -u -p -r1.116 aucat.1
--- aucat/aucat.1   22 Apr 2020 05:37:00 -  1.116
+++ aucat/aucat.1   21 Dec 2021 17:53:13 -
@@ -88,7 +88,7 @@ Increase log verbosity.
 .It Fl e Ar enc
 Encoding of the audio file.
 The default is
-.Va s16 .
+.Va s24 .
 Encoding names use the following scheme: signedness
 .Po
 .Va s
Index: aucat/aucat.c
===
RCS file: /cvs/src/usr.bin/aucat/aucat.c,v
retrieving revision 1.177
diff -u -p -u -p -r1.177 aucat.c
--- aucat/aucat.c   12 Jan 2021 15:46:53 -  1.177
+++ aucat/aucat.c   21 Dec 2021 17:53:13 -
@@ -1393,7 +1393,11 @@ main(int argc, char **argv)
rate = DEFAULT_RATE;
cmin = 0;
cmax = 1;
-   aparams_init();
+   par.bits = ADATA_BITS;
+   par.bps = APARAMS_BPS(par.bits);
+   par.le = ADATA_LE;
+   par.sig = 1;
+   par.msb = 1;
hdr = AFILE_HDR_AUTO;
n_flag = 0;
port = NULL;
Index: aucat/dsp.h
===
RCS file: /cvs/src/usr.bin/aucat/dsp.h,v
retrieving revision 1.8
diff -u -p -u -p -r1.8 dsp.h
--- aucat/dsp.h 25 May 2021 08:06:12 -  1.8
+++ aucat/dsp.h 21 Dec 2021 17:53:13 -
@@ -25,28 +25,14 @@
  * boundary is excluded. We represent them as signed fixed point numbers
  * of ADATA_BITS. We also assume that 2^(ADATA_BITS - 1) fits in a int.
  */
-#ifndef ADATA_BITS
-#define ADATA_BITS  

Re: rpki-client, stop using size_t for ids

[Job Snijders]

On Tue, Dec 21, 2021 at 07:00:03PM +0100, Claudio Jeker wrote:
> For some reasons various ids were stored as size_t (probably because once
> they used to be the index in an array). This is just silly and annoyed me
> for long enough. I think this fixes all of them.
> 
> While there also stop using size_t for maxlength of a prefix. Everywhere
> else the code uses just a unsigned char for that so do it there as well.
> Shuffle struct a little bit in hope for better packing.
> 
> Seems to work with RRDP and RSYNC.

print.c line 155 may need a small change too (-Wformat error)

other than that, OK job@

more rpki-client cleanup

[Claudio Jeker]

In the roa parser the handling of maxlen is overly complex.
Just set maxlen to addr.prefixlen before parsing the maxlength option.
If present it will override maxlen with the new value and with that the
ternary confusion at the end can be removed.

-- 
:wq Claudio

Index: roa.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/roa.c,v
retrieving revision 1.33
diff -u -p -r1.33 roa.c
--- roa.c   3 Dec 2021 12:56:19 -   1.33
+++ roa.c   21 Dec 2021 18:09:27 -
@@ -50,7 +50,7 @@ roa_parse_addr(const ASN1_OCTET_STRING *
size_t   dsz = os->length;
int  rc = 0;
const ASN1_TYPE *t;
-   const ASN1_INTEGER  *maxlength = NULL;
+   const ASN1_INTEGER  *maxlength;
long maxlen;
struct ip_addr   addr;
struct roa_ip   *res;
@@ -83,6 +83,7 @@ roa_parse_addr(const ASN1_OCTET_STRING *
"invalid IP address", p->fn);
goto out;
}
+   maxlen = addr.prefixlen;
 
if (sk_ASN1_TYPE_num(seq) == 2) {
t = sk_ASN1_TYPE_value(seq, 1);
@@ -115,7 +116,7 @@ roa_parse_addr(const ASN1_OCTET_STRING *
 
res->addr = addr;
res->afi = afi;
-   res->maxlength = (maxlength == NULL) ? addr.prefixlen : maxlen;
+   res->maxlength = maxlen;
ip_roa_compose_ranges(res);
 
rc = 1;

rpki-client, stop using size_t for ids

[Claudio Jeker]

For some reasons various ids were stored as size_t (probably because once
they used to be the index in an array). This is just silly and annoyed me
for long enough. I think this fixes all of them.

While there also stop using size_t for maxlength of a prefix. Everywhere
else the code uses just a unsigned char for that so do it there as well.
Shuffle struct a little bit in hope for better packing.

Seems to work with RRDP and RSYNC.
-- 
:wq Claudio

Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.98
diff -u -p -r1.98 extern.h
--- extern.h25 Nov 2021 14:03:40 -  1.98
+++ extern.h21 Dec 2021 17:26:16 -
@@ -179,10 +179,10 @@ struct mft {
  */
 struct roa_ip {
enum afi afi; /* AFI value */
-   size_t   maxlength; /* max length or zero */
+   struct ip_addr   addr; /* the address prefix itself */
unsigned charmin[16]; /* full range minimum */
unsigned charmax[16]; /* full range maximum */
-   struct ip_addr   addr; /* the address prefix itself */
+   unsigned charmaxlength; /* max length or zero */
 };
 
 /*
@@ -498,8 +498,8 @@ void proc_rrdp(int);
 
 /* Repository handling */
 int filepath_add(struct filepath_tree *, char *);
-voidrrdp_save_state(size_t, struct rrdp_session *);
-int rrdp_handle_file(size_t, enum publish_type, char *,
+voidrrdp_save_state(unsigned int, struct rrdp_session *);
+int rrdp_handle_file(unsigned int, enum publish_type, char *,
char *, size_t, char *, size_t);
 char   *repo_filename(const struct repo *, const char *);
 struct repo*ta_lookup(int, struct tal *);
@@ -508,15 +508,15 @@ intrepo_queued(struct repo *, struct 
 voidrepo_cleanup(struct filepath_tree *);
 voidrepo_free(void);
 
-voidrsync_finish(size_t, int);
-voidhttp_finish(size_t, enum http_result, const char *);
-voidrrdp_finish(size_t, int);
-
-voidrsync_fetch(size_t, const char *, const char *);
-voidhttp_fetch(size_t, const char *, const char *, int);
-voidrrdp_fetch(size_t, const char *, const char *,
+voidrsync_finish(unsigned int, int);
+voidhttp_finish(unsigned int, enum http_result, const char *);
+voidrrdp_finish(unsigned int, int);
+
+voidrsync_fetch(unsigned int, const char *, const char *);
+voidhttp_fetch(unsigned int, const char *, const char *, int);
+voidrrdp_fetch(unsigned int, const char *, const char *,
struct rrdp_session *);
-voidrrdp_http_done(size_t, enum http_result, const char *);
+voidrrdp_http_done(unsigned int, enum http_result, const char *);
 
 int repo_next_timeout(int);
 voidrepo_check_timeout(void);
Index: http.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v
retrieving revision 1.50
diff -u -p -r1.50 http.c
--- http.c  10 Nov 2021 09:13:30 -  1.50
+++ http.c  21 Dec 2021 17:19:29 -
@@ -140,7 +140,7 @@ struct http_request {
char*host;
char*port;
const char  *path;  /* points into uri */
-   size_t   id;
+   unsigned int id;
int  outfd;
int  redirect_loop;
 };
@@ -150,7 +150,7 @@ TAILQ_HEAD(http_req_queue, http_request)
 static struct http_conn_list   active = LIST_HEAD_INITIALIZER(active);
 static struct http_conn_list   idle = LIST_HEAD_INITIALIZER(idle);
 static struct http_req_queue   queue = TAILQ_HEAD_INITIALIZER(queue);
-static size_t http_conn_count;
+static unsigned int http_conn_count;
 
 static struct msgbuf msgq;
 static struct sockaddr_storage http_bindaddr;
@@ -159,10 +159,10 @@ static uint8_t *tls_ca_mem;
 static size_t tls_ca_size;
 
 /* HTTP request API */
-static voidhttp_req_new(size_t, char *, char *, int, int);
+static voidhttp_req_new(unsigned int, char *, char *, int, int);
 static voidhttp_req_free(struct http_request *);
-static voidhttp_req_done(size_t, enum http_result, const char *);
-static voidhttp_req_fail(size_t);
+static voidhttp_req_done(unsigned int, enum http_result, const char *);
+static voidhttp_req_fail(unsigned int);
 static int http_req_schedule(struct http_request *);
 
 /* HTTP connection API */
@@ -509,7 +509,8 @@ http_resolv(struct addrinfo **res, const
  * Create and queue a new request.
  */
 static void
-http_req_new(size_t id, char *uri, char *modified_since, int count, int outfd)
+http_req_new(unsigned int id, char *uri, char *modified_since, int count,
+int outfd)
 {
struct http_request *req;
   

rpki-client simplify code a bit

[Claudio Jeker]

The limiter for repository count under a TA only makes sense for
repositories referenced from certs but less so for the actual TA. So
remove the logic from ta_lookup() and friends and make the code simpler.
There is no risk in doing so since there is only one TA and one
ta_lookup() done per TAL and the MAX_REPO_PER_TAL check can never trigger.

-- 
:wq Claudio

Index: repo.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v
retrieving revision 1.15
diff -u -p -r1.15 repo.c
--- repo.c  7 Dec 2021 12:46:47 -   1.15
+++ repo.c  21 Dec 2021 15:20:26 -
@@ -381,7 +381,7 @@ ta_fetch(struct tarepo *tr)
 }
 
 static struct tarepo *
-ta_get(struct tal *tal, int nofetch)
+ta_get(struct tal *tal)
 {
struct tarepo *tr;
 
@@ -405,7 +405,7 @@ ta_get(struct tal *tal, int nofetch)
tal->urisz = 0;
tal->uri = NULL;
 
-   if (noop || nofetch) {
+   if (noop) {
tr->state = REPO_DONE;
logx("ta/%s: using cache", tr->descr);
/* there is nothing in the queue so no need to flush */
@@ -1087,7 +1087,6 @@ struct repo *
 ta_lookup(int id, struct tal *tal)
 {
struct repo *rp;
-   int  nofetch = 0;
 
/* Look up in repository table. (Lookup should actually fail here) */
SLIST_FOREACH(rp, , entry) {
@@ -1099,13 +1098,7 @@ ta_lookup(int id, struct tal *tal)
if ((rp->repouri = strdup(tal->descr)) == NULL)
err(1, NULL);
 
-   if (++talrepocnt[id] >= MAX_REPO_PER_TAL) {
-   if (talrepocnt[id] == MAX_REPO_PER_TAL)
-   warnx("too many repositories under %s", tals[id]);
-   nofetch = 1;
-   }
-
-   rp->ta = ta_get(tal, nofetch);
+   rp->ta = ta_get(tal);
 
return rp;
 }

sysctl diskinit tailq foreach

[Alexander Bluhm]

Hi,

I would like to use TAILQ_FOREACH to traverse the disk list.
Code is easier to read.

ok?

bluhm

Index: kern/kern_sysctl.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.396
diff -u -p -r1.396 kern_sysctl.c
--- kern/kern_sysctl.c  30 Oct 2021 23:24:48 -  1.396
+++ kern/kern_sysctl.c  21 Dec 2021 14:23:02 -
@@ -121,7 +121,6 @@
 extern struct forkstat forkstat;
 extern struct nchstats nchstats;
 extern int nselcoll, fscale;
-extern struct disklist_head disklist;
 extern fixpt_t ccpu;
 extern long numvnodes;
 extern int allowdt;
@@ -2132,14 +2131,14 @@ sysctl_diskinit(int update, struct proc 
struct diskstats *sdk;
struct disk *dk;
const char *duid;
-   int i, tlen, l;
+   int error, tlen, l;
 
-   if ((i = rw_enter(_disklock, RW_WRITE|RW_INTR)) != 0)
-   return i;
+   if ((error = rw_enter(_disklock, RW_WRITE|RW_INTR)) != 0)
+   return error;
 
if (disk_change) {
-   for (dk = TAILQ_FIRST(), tlen = 0; dk;
-   dk = TAILQ_NEXT(dk, dk_link)) {
+   tlen = 0;
+   TAILQ_FOREACH(dk, , dk_link) {
if (dk->dk_name)
tlen += strlen(dk->dk_name);
tlen += 18; /* label uid + separators */
@@ -2159,8 +2158,9 @@ sysctl_diskinit(int update, struct proc 
disknameslen = tlen;
disknames[0] = '\0';
 
-   for (dk = TAILQ_FIRST(), i = 0, l = 0; dk;
-   dk = TAILQ_NEXT(dk, dk_link), i++) {
+   l = 0;
+   sdk = diskstats;
+   TAILQ_FOREACH(dk, , dk_link) {
duid = NULL;
if (dk->dk_label && !duid_iszero(dk->dk_label->d_uid))
duid = duid_format(dk->dk_label->d_uid);
@@ -2168,7 +2168,6 @@ sysctl_diskinit(int update, struct proc 
dk->dk_name ? dk->dk_name : "",
duid ? duid : "");
l += strlen(disknames + l);
-   sdk = diskstats + i;
strlcpy(sdk->ds_name, dk->dk_name,
sizeof(sdk->ds_name));
mtx_enter(>dk_mtx);
@@ -2182,6 +2181,7 @@ sysctl_diskinit(int update, struct proc 
sdk->ds_timestamp = dk->dk_timestamp;
sdk->ds_time = dk->dk_time;
mtx_leave(>dk_mtx);
+   sdk++;
}
 
/* Eliminate trailing comma */
@@ -2190,9 +2190,8 @@ sysctl_diskinit(int update, struct proc 
disk_change = 0;
} else if (update) {
/* Just update, number of drives hasn't changed */
-   for (dk = TAILQ_FIRST(), i = 0; dk;
-   dk = TAILQ_NEXT(dk, dk_link), i++) {
-   sdk = diskstats + i;
+   sdk = diskstats;
+   TAILQ_FOREACH(dk, , dk_link) {
strlcpy(sdk->ds_name, dk->dk_name,
sizeof(sdk->ds_name));
mtx_enter(>dk_mtx);
@@ -2206,6 +2205,7 @@ sysctl_diskinit(int update, struct proc 
sdk->ds_timestamp = dk->dk_timestamp;
sdk->ds_time = dk->dk_time;
mtx_leave(>dk_mtx);
+   sdk++;
}
}
rw_exit_write(_disklock);

Re: document patch(1) not supporting binary diffs

[Jason McIntyre]

On Tue, Dec 21, 2021 at 09:43:27AM +0100, Ingo Schwarze wrote:
> Jason McIntyre wrote on Mon, Dec 20, 2021 at 04:13:19PM +:
> 
> > i'm ok with your diff
> 
> Thanks for checking.
> 
> > but it is slightly misleading in context of
> > reading from stdin. i suppose that is no biggie.
> 
> I think i see why you say so.
> 
> Speaking *formally*, what we usually do is describing what the utility
> does by default in the first sentence, then explain how command line
> options modify that behaviour below "The options are as follows".
> We could do that here, somewhat like this:
> 
>   patch reads a text file called the "patch file" from standard input,
>   containing any of the four forms ... yadayada ...
>   and applies those differences to an original text file ...
> 
> Besides, usually, we would *not* show a SYNOPSIS line
> 
>   patch  
> In this particular case, however, typing "patch  maybe "patch -i patchfile" is so dominant in practice that focussing
> on it seems to help users more than adhering to formal practices.
> 
> I think calling this "misleading" is a bit strong because very often,
> the first sentence in our manual pages describes the default or most
> important mode of operation and subsequent sentences modify that by
> detailing other modes.  In this case, not only does the next sentence
> say what happens if no patch file is specified, but we also have the
> "patch  

hi.

my concern was that we would change a piece of text that talked
generally about patch files (without overly describing the format) into
a specific reference to an exact file (.Ar patchfile). the former does
not generally exclude stdin, but the latter seemed to do so. or at least
leave some ambiguity.

so i hoped to avoid the issue with a simpler fix. still, as noted, i
didn;t feel it to be overly important since, as you say, the document
does then go on to explain things. and we do usually talk about how
things work in general, then flesh out differences.

> > it would be simpler to just s/patch file/text file/ maybe?
> 
> True, that would be simpler and not outright wrong.  But in normal
> operation, patch(1) deals with many text files, so i worry that
> people might get confused as to which text file we are talking about.
> It seems valuable for the first sentence to make it clear that one
> among all these files is special and to introduce the term "patch file"
> or ".Ar patchfile".
> 

heh, so the opposite of what i'm describing.

> So to first get the issue reported by chrisz@ out of the way, i
> committed the patch as-is.
> 

sure, makes sense

> We could say
> 
>   .Nm
>   takes a text file called the
>   .Sq patch file
>   containing any of the four forms ...
> 
> Do you think that is better, like in the patch below?
> 

yes, because it seems to address all our points. on the other hand, i
can;t help but feel that the original wording was better, even if it has
the issue about not being clear that it should be a text file. it was
simpler and easier to understand.

would it work to reinstate that original wording ("patch will take a
patch file"), but somehow strongarm the second sentence ("If
patchfile is omitted") into one describing both that it shold be a text
file, and what happens if it is omitted?

i.e. leave the original wording of sentence 1, but adjust that of
sentence 2.

what do you think?
jmc

> Yours,
>   Ingo
> 
> 
> Index: patch.1
> ===
> RCS file: /cvs/src/usr.bin/patch/patch.1,v
> retrieving revision 1.34
> diff -u -r1.34 patch.1
> --- patch.1   21 Dec 2021 08:07:20 -  1.34
> +++ patch.1   21 Dec 2021 08:37:54 -
> @@ -47,8 +47,8 @@
>  .Pf \*(Lt Ar patchfile
>  .Sh DESCRIPTION
>  .Nm
> -takes the text file
> -.Ar patchfile
> +takes a text file called the
> +.Dq patch file
>  containing any of the four forms of difference
>  listing produced by the
>  .Xr diff 1
> @@ -56,7 +56,7 @@
>  producing a patched version.
>  If
>  .Ar patchfile
> -is omitted, or is a hyphen, the patch will be read from the standard input.
> +is omitted or is a hyphen, the patch file is read from the standard input.
>  .Pp
>  .Nm
>  will attempt to determine the type of the diff listing, unless overruled by a
> 

Re: document patch(1) not supporting binary diffs

[Ingo Schwarze]

Hi Theo,

Theo de Raadt wrote on Mon, Dec 20, 2021 at 10:37:00AM -0700:
> Ingo Schwarze  wrote:

>> The patch(1) manual talks about "lines" throughout,
>> and for binary files, a concept of "lines" does not even exist.

> That is a bit strong.  Some utilities designed for "text files" have no
> problem being both 8-bit clean and very-long-line capable.  For example,
> you can use emacs to edit a /bsd.  Add a space at the start of the file,
> save it.  re-edit the file, delete the space, and you get the same
> result.  I changed the internals of mg decades ago to also be 8-bit
> clean + very-long-line capable.

Yes, i agree such extensions are often useful.

> I think in POSIX this concept of "text
> file" is very weakly defined -- there are a variety of line-length
> issues, weird issues relating to \n and \r translation, obvious
> inability to handle embeded NUL etc, etc etc, and even end-of-file
> newline termination.
> 
> It is a bit of a copout to say "text file", when the truth really is is
> "imperfect content handling".  OTOH, adding perfect handling in many unix
> programs would be basically impossible, the complexity would be pretty high.
> 
> It may even be that the patch inputfile format cannot handle arbitrary data.
> It certainly has newline followed by '+', '-', or ' ' and ending with '\n'
> as context tracking, so it might be impossible.
 
>> The problem arises from the manual failing to mention that patch(1)
>> operates on text files.

> Yes, I am OK with it saying so.

>> Many standard utilities operate on text files
>> only, the concept of a text-file is both well-known and defined by
>> POSIX, so there is no need to re-explain what that means in individual
>> pages.

> Nope, I think POSIX fails to create a good definition.

Admittedly, the POSIX definition is a bit weak in some respects,
but all the same, it is usable as a minmium requirement that many
standard utilities have in common.

> Look at the
> recent commit to uniq.c, you even commented on it.  patch can handle files
> without trailing \n, right?   If the POSIX definition was real, patch would
> probably need to fail upon those files, creating other problems.

If POSIX says "The foobar utility operates on text files", that does not
mean that it requires foobar to fail if the input is not a text file.
It merely means that the behaviour is unspecified in that case.

It may occasionally be useful to add statements similar to the following
to the STANDARDS section:

  As an extension to that specification,

  foobar can handle files containing NUL bytes
or
  foobar can handle files containing bytes that do not form characters
or
  foobar can handle lines longer than LINE_MAX bytes
or
  foobar can handle files lacking the terminating newline character
  on the last line

Only in cases where such an extension is useful for some practical
purpose and we are willing to maintain it, of course.

Yours,
  Ingo

Re: document patch(1) not supporting binary diffs

[Ingo Schwarze]

Jason McIntyre wrote on Mon, Dec 20, 2021 at 04:13:19PM +:

> i'm ok with your diff

Thanks for checking.

> but it is slightly misleading in context of
> reading from stdin. i suppose that is no biggie.

I think i see why you say so.

Speaking *formally*, what we usually do is describing what the utility
does by default in the first sentence, then explain how command line
options modify that behaviour below "The options are as follows".
We could do that here, somewhat like this:

  patch reads a text file called the "patch file" from standard input,
  containing any of the four forms ... yadayada ...
  and applies those differences to an original text file ...

Besides, usually, we would *not* show a SYNOPSIS line

  patch  it would be simpler to just s/patch file/text file/ maybe?

True, that would be simpler and not outright wrong.  But in normal
operation, patch(1) deals with many text files, so i worry that
people might get confused as to which text file we are talking about.
It seems valuable for the first sentence to make it clear that one
among all these files is special and to introduce the term "patch file"
or ".Ar patchfile".

So to first get the issue reported by chrisz@ out of the way, i
committed the patch as-is.

We could say

  .Nm
  takes a text file called the
  .Sq patch file
  containing any of the four forms ...

Do you think that is better, like in the patch below?

Yours,
  Ingo


Index: patch.1
===
RCS file: /cvs/src/usr.bin/patch/patch.1,v
retrieving revision 1.34
diff -u -r1.34 patch.1
--- patch.1 21 Dec 2021 08:07:20 -  1.34
+++ patch.1 21 Dec 2021 08:37:54 -
@@ -47,8 +47,8 @@
 .Pf \*(Lt Ar patchfile
 .Sh DESCRIPTION
 .Nm
-takes the text file
-.Ar patchfile
+takes a text file called the
+.Dq patch file
 containing any of the four forms of difference
 listing produced by the
 .Xr diff 1
@@ -56,7 +56,7 @@
 producing a patched version.
 If
 .Ar patchfile
-is omitted, or is a hyphen, the patch will be read from the standard input.
+is omitted or is a hyphen, the patch file is read from the standard input.
 .Pp
 .Nm
 will attempt to determine the type of the diff listing, unless overruled by a