Re: Upgrade to OpenBSD 7.5 broke the bsd game of cribbage
Otto Moerbeek wrote:
> On Wed, May 29, 2024 at 08:05:14AM +0200, Otto Moerbeek wrote:
>
> > On Mon, May 27, 2024 at 09:21:34PM -0500, Don Wilburn wrote:
> >
> > > Dear OpenBSD,
> > >
> > > I recently upgraded from version 7.4 to 7.5. This broke the old cribbage
> > > game. This is included with OpenBSD, if you choose to install the games.
> > >
> > > I'm not a programmer, but I promise you this happened because ncurses was
> > > updated from version 5.7 to 6.4
> > >
> > > The problem:
> > >
> > > Normally the game gives prompts for play options and cards. It's supposed
> > > to leave the prompt after the response, then advance to a new line. This
> > > gives a brief history of selections
> > >
> > > Now, starting with the third prompt (cut the cards), the prompts
> > > disappear
> > > when a response key is pressed. This ruins the game. The effect is
> > > obvious,
> > > even if you don't know how to play cribbage.
> > >
> > > It would be even more obvious if you have an older system to compare with
> > > a
> > > current v7.5 system.
> > >
> > > This happened to linux bsd-games many years ago. A search will indicate
> > > that I filed this same bug with Gentoo linux over 9 years ago. Linux
> > > classic bsd-games has been unmaintained since before that time. This is
> > > where I observed that the bug happened with a ncurses update. Nobody
> > > pursued the solution.
> > >
> > > I don't have the skills to butcher the game code to work with with the
> > > update of ncurses. Likewise, I don't know how to use a debugger or write
> > > a
> > > sample program to replicate the effect. I can't demonstrate WHY ncurses
> > > is
> > > the problem. Maybe it's the C compiler's fault?
> > >
> > > I still play this obsolete command line game. It's nostalgia, I guess. I
> > > know OpenBSD developers have really important things to maintain. If
> > > someone could spare some time for this little bug, I'd be happy. Maybe it
> > > could be delegated to a student?
> > >
> > > Thanks for reading, DW
> > >
> >
> > One remains a student forever.
> >
> > Try this, it does not try to cut corners with switching windows.
>
> No response from the original reporter.
>
> Is anybody else interested in testing/reviewing?
>
> -Otto
Hi Otto,
I can confirm the behaviour reported by Don Wilburn and that your diff
fixes the issue. I have no idea how to play cribbage, but as Don noted,
the impact is obvious.
FWIW, your fix makes sense to me. A changed line runs to 86 columns as
annotated inline but in the cribbage tree there seems to be instances
where its reflowed to fit within 80 and others where it doesn't.
> >
> > Index: io.c
> > ===
> > RCS file: /home/cvs/src/games/cribbage/io.c,v
> > diff -u -p -r1.22 io.c
> > --- io.c10 Jan 2016 13:35:09 - 1.22
> > +++ io.c29 May 2024 06:00:03 -
> > @@ -505,14 +505,11 @@ get_line(void)
> > {
> > size_t pos;
> > int c, oy, ox;
> > - WINDOW *oscr;
> >
> > - oscr = stdscr;
> > - stdscr = Msgwin;
> > - getyx(stdscr, oy, ox);
> > - refresh();
> > + getyx(Msgwin, oy, ox);
> > + wrefresh(Msgwin);
> > /* loop reading in the string, and put it in a temporary buffer */
> > - for (pos = 0; (c = readchar()) != '\n'; clrtoeol(), refresh()) {
> > + for (pos = 0; (c = readchar()) != '\n'; wclrtoeol(Msgwin),
> > wrefresh(Msgwin)) {
The above line runs to 86 columns, perhaps:
for (pos = 0; (c = readchar()) != '\n';
wclrtoeol(Msgwin), wrefresh(Msgwin)) {
> > if (c == -1)
> > continue;
> > if (c == ' ' && (pos == 0 || linebuf[pos - 1] == ' '))
> > @@ -522,13 +519,13 @@ get_line(void)
> > int i;
> > pos--;
> > for (i = strlen(unctrl(linebuf[pos])); i; i--)
> > - addch('\b');
> > + waddch(Msgwin, '\b');
> > }
> > continue;
> > }
> > if (c == killchar()) {
> > pos = 0;
> > - move(oy, ox);
> > + wmove(Msgwin, oy, ox);
> > continue;
> > }
> > if (pos >= LINESIZE - 1 || !(isalnum(c) || c == ' ')) {
> > @@ -538,12 +535,11 @@ get_line(void)
> > if (islower(c))
> > c = toupper(c);
> > linebuf[pos++] = c;
> > - addstr(unctrl(c));
> > + waddstr(Msgwin, unctrl(c));
> > Mpos++;
> > }
> > while (pos < sizeof(linebuf))
> > linebuf[pos++] = '\0';
> > - stdscr = oscr;
> > return (linebuf);
> > }
> >
> >
--
Mark Jamsek <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
Re: possible underflow (wrap) in tcpdump/print-domain.c
goto trunc;
> > > - while (nscount-- && cp < snapend) {
> > > + while (nscount-- > 0 && cp < snapend) {
> > > putchar(',');
> > > if ((cp = ns_rprint(cp, bp, is_mdns))
> > > == NULL)
> > > goto trunc;
> > > @@ -723,11 +723,11 @@ ns_print(const u_char *bp, u_int length,
> > > }
> > > if (nscount > 0)
> > > goto trunc;
> > > - if (cp < snapend && arcount--) {
> > > + if (cp < snapend && arcount-- > 0) {
> > > printf(" ar:");
> > > if ((cp = ns_rprint(cp, bp, is_mdns)) == NULL)
> > > goto trunc;
> > > - while (cp < snapend && arcount--) {
> > > + while (cp < snapend && arcount-- > 0) {
> > > putchar(',');
> > > if ((cp = ns_rprint(cp, bp, is_mdns))
> > > == NULL)
> > > goto trunc;
> > >
> > >
> >
> > While not pretty there is nothing wrong with the current code.
> >
> > All 4 variables qdcount, ancount, nscount, arcount are only decremented by
> > one and always checked for 0. So there is no way any of the 4 values
> > become negative. Also the EXTRACT_16BITS() puts a uint16_t into an int
> > which never overflows on OpenBSD. int is always 32bit on OpenBSD.
> >
> > Still it may make sense to apply the diff just to be explicit about the
> > count values.
> >
> > --
> > :wq Claudio
>
> Hi Claudio,
>
> I ask you to look closer. Perhaps I didn't explain the problem best.
> Let's take variable qdcount:
>
>603 while (qdcount--) {
>
> After this while() qdcount wraps, and that's fine if it isn't reused! But in
> the same function below it is tested again (at which point it is negative).
This second use of qdcount is in the `else` block; that is, in this
routine, qdcount will only be used in the above `if (DNS_QR(np))` block,
or here--not in both. So I think Claudio is correct.
>686 if (qdcount--) {
>
> and
>
>690 while (cp < snapend && qdcount--) {
>
>
> Best Regards,
> -peter
>
--
Mark Jamsek
GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
Re: mail(1) "save" command straying from POSIX for missing filename
On 22-12-18 09:29PM, Jason McIntyre wrote:
> On Fri, Dec 16, 2022 at 02:21:41AM +, Tim Chase wrote:
> > According to the POSIX definitions for mail(1) & mailx(1), the
> > (s)ave command should save to "mbox" if the filename is not specified
> >
> > > Save the specified messages in the file named by the pathname
> > > file, or the mbox if the file argument is omitted
> >
> > (newer spec)
> > https://pubs.opengroup.org/onlinepubs/9699919799/utilities/mailx.html#tag_20_75_13_33
> >
> > > s [file]
> > > Save the message in the named file (mbox is default).
> >
> > (older spec)
> > https://pubs.opengroup.org/onlinepubs/7908799/xcu/mail.html#tag_001_014_1339
> >
> >
> >
> > However, when exercising this functionality, mail(1) on OpenBSD
> > (also tested on FreeBSD where the same issue manifests[1]) doesn't
> > support this:
> >
> > demo$ echo test | mail -s "test" demo # send self a message
> > demo$ mail
> > Mail version 8.1 6/6/93. Type ? for help.
> > "/var/mail/demo": 1 message 1 new
> > >N 1 d...@localhost.my.do Thu Dec 15 19:34 19/775 "test"
> > & s
> > No file specified.
> >
> > While I'm not positive on the solution, I think it involves tweaking
> > the save1() function in src/usr.bin/mail/cmd2.c such that instead
> > of failing if it can't snarf(), it should set `file` to "mbox" or
> > "&" so that expand() points to the mbox as required by POSIX.
> >
> > -tkc
> >
> > [1]
> > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=268402
> >
>
> hi.
>
> currently mail(1) has these entries in FILES:
>
> FILES
>/var/mail/* post office (unless overridden
>by the MAIL environment
>variable)
>~/mbox user's old mail
>
> isn;t it the case that openbsd uses mailboxes in /var/mail by default,
> instead of ~/mbox, as displayed?
>
> it seems that mail(1) is really out of date regarding default mail spool
> entries, but i may well have misunderstood the situation. once it's
> clear, i can see if we need a code fix (out of my hands) or doc fix.
>
> jmc
I agree with Brian and think behaviour comports with mail(1). Unexamined
mail remains in the post office (i.e., /var/mail/*), and examined mail,
as noted by Brian, is deposited to the user's mbox file (i.e., ~/mbox):
You can end a mail session with the quit (q) command. Messages
which have been examined go to your mbox file unless they have
been deleted, in which case they are discarded. Unexamined
messages go back to the post office (see the -f option above).
If the session is aborted with e(x)it, however, changes are discarded
thus mail remains unexamined and left in the post office.
Regarding the OP's case, (s)ave is also consistent with mail(1) except
that line and char count are not currently echoed (the below diff adds
this to the output):
save (s) Takes a message list and a filename and appends each message
in turn to the end of the file. The filename in quotes,
followed by the line count and character count is echoed on the
user's terminal.
I guess it's a question of whether we want to change this and make it
comply with POSIX so that "s" with no args saves the current message to
the user's mbox file as the diff upthread does. I think it's handy, but
that behaviour might have been omitted on purpose. And, tbh, "s" isn't
really that much more convenient than "s &".
diff d956567b8a83e77dcbaa40d1038b81c18ca02b19
0628dc730fed3c76f8e1cec17dd1c90c3a58aa75
commit - d956567b8a83e77dcbaa40d1038b81c18ca02b19
commit + 0628dc730fed3c76f8e1cec17dd1c90c3a58aa75
blob - 54b30bc153cd53765a7d15eb0430246c9b46fb17
blob + 7a0650ddcf690e3b2f61550007cf40819b308f94
--- usr.bin/mail/cmd2.c
+++ usr.bin/mail/cmd2.c
@@ -146,6 +146,8 @@ save1(char *str, int mark, char *cmd, struct ignoretab
{
struct message *mp;
char *file, *disp;
+ off_t sz = 0;
+ int nlines = 0;
int f, *msgvec, *ip;
FILE *obuf;
@@ -182,6 +184,8 @@ save1(char *str, int mark, char *cmd, struct ignoretab
(void)Fclose(obuf);
return(1);
}
+ nlines += mp->m_lines;
+ sz += mp->m_size;
if (mark)
mp->m_flag |= MSAVED;
}
@@ -189,7 +193,7 @@ save1(char *str, int mark, char *cmd, struct ignoretab
if (ferror(obuf))
warn("%s", file);
(void)Fclose(obuf);
- printf("%s\n", disp);
+ printf("%s %d/%lld\n", disp, nlines, (long long)sz);
return(0);
}
--
Mark Jamsek
GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
signature.asc
Description: PGP signature
Re: mail(1) "save" command straying from POSIX for missing filename
On 22-12-16 02:21AM, Tim Chase wrote:
> According to the POSIX definitions for mail(1) & mailx(1), the
> (s)ave command should save to "mbox" if the filename is not specified
>
> ...
>
> However, when exercising this functionality, mail(1) on OpenBSD
> (also tested on FreeBSD where the same issue manifests[1]) doesn't
> support this:
>
> demo$ echo test | mail -s "test" demo # send self a message
> demo$ mail
> Mail version 8.1 6/6/93. Type ? for help.
> "/var/mail/demo": 1 message 1 new
> >N 1 d...@localhost.my.do Thu Dec 15 19:34 19/775 "test"
> & s
> No file specified.
Current behaviour comports with the mail(1) manual page, so support for
this may be intentionally elided; I'm not sure. In either case, here's
a minimal diff making the change.
Index: cmd2.c
===
RCS file: /cvs/src/usr.bin/mail/cmd2.c,v
retrieving revision 1.22
diff -u -p -r1.22 cmd2.c
--- cmd2.c 16 Oct 2015 17:56:07 - 1.22
+++ cmd2.c 16 Dec 2022 12:59:21 -
@@ -139,6 +139,7 @@ copycmd(void *v)
/*
* Save/copy the indicated messages at the end of the passed file name.
+ * If no file name is specified, default to user mbox.
* If mark is true, mark the message "saved."
*/
int
@@ -208,10 +209,11 @@ swrite(void *v)
/*
* Snarf the file from the end of the command line and
* return a pointer to it. If there is no file attached,
- * just return NULL. Put a null in front of the file
+ * return the mbox file. Put a null in front of the file
* name so that the message list processing won't see it,
- * unless the file name is the only thing on the line, in
- * which case, return 0 in the reference flag variable.
+ * unless the file name is the only thing on the line, or
+ * no file was attached, in which case, return 0 in the
+ * reference flag variable.
*/
char *
snarf(char *linebuf, int *flag)
@@ -234,8 +236,8 @@ snarf(char *linebuf, int *flag)
while (cp > linebuf && !isspace((unsigned char)*cp))
cp--;
if (*cp == '\0') {
- puts("No file specified.");
- return(NULL);
+ *flag = 0;
+ return(expand("&"));
}
if (isspace((unsigned char)*cp))
*cp++ = 0;
Index: mail.1
===
RCS file: /cvs/src/usr.bin/mail/mail.1,v
retrieving revision 1.83
diff -u -p -r1.83 mail.1
--- mail.1 31 Mar 2022 17:27:25 - 1.83
+++ mail.1 16 Dec 2022 12:59:22 -
@@ -633,6 +633,9 @@ retained fields.
.Pq Ic s
Takes a message list and a filename and appends each message in
turn to the end of the file.
+If filename is omitted, the
+.Ar mbox
+file is used.
The filename in quotes, followed by the line
count and character count is echoed on the user's terminal.
.It Ic saveignore
--
Mark Jamsek
GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
signature.asc
Description: PGP signature
