Re: What is ieee80211com->ic_ibss_chan?
On Monday, August 8, 2022 7:40:19 PM EDT Jonathan Gray wrote: > On Mon, Aug 08, 2022 at 11:40:17AM -0400, Farhan Khan wrote: > > Hi all, > > > > I am trying to understand the different channels in `struct ieee80211com`. > > I see 2 types of ieee80211_channel values: * ic_ibss_chan > > * ic_des_chan - Desired channel, pretty straight forward > > > > What is ic_ibss_chan used for? My initial guess was that it was the > > "current channel", but given that other implementations of net80211 > > (NetBSD, FreeBSD, Illumos) have `ic_curchan`, it does not seem to be the > > current channel. > > > > What is ic_ibss_channel used for? And where is the current channel > > information stored? > refer to the definitions in the 802.11 spec > independent basic service set (IBSS) also known as ad hoc, no AP > basic service set (BSS) also known as infrastructure > > for channel see SIOCS80211CHANNEL and ni_chan in struct ieee80211_node Thank you for your reply! Just to make sure I understand properly, `ic_ibss_chan` is used for Ad hoc or monitor mode, whereas infrastructure mode (BSS) uses `ieee80211_node` which specifies the channel. Thank you! - Farhan
Re: What is ieee80211com->ic_ibss_chan?
On Mon, Aug 08, 2022 at 11:40:17AM -0400, Farhan Khan wrote: > Hi all, > > I am trying to understand the different channels in `struct ieee80211com`. I > see 2 types of ieee80211_channel values: > * ic_ibss_chan > * ic_des_chan - Desired channel, pretty straight forward > > What is ic_ibss_chan used for? My initial guess was that it was the "current > channel", but given that other implementations of net80211 (NetBSD, FreeBSD, > Illumos) have `ic_curchan`, it does not seem to be the current channel. > > What is ic_ibss_channel used for? And where is the current channel > information stored? refer to the definitions in the 802.11 spec independent basic service set (IBSS) also known as ad hoc, no AP basic service set (BSS) also known as infrastructure for channel see SIOCS80211CHANNEL and ni_chan in struct ieee80211_node
Re: ipv6 fragment checksum
On Sun, Aug 07, 2022 at 12:41:29AM +0200, Alexander Bluhm wrote: > Hi, > > If interface drivers have enabled transmit offloading for the payload > checksum , IPv6 fragments contain invalid checksum. For fragments > the protocol checksum has to be calculated before fragmentation. > Hardware cannot do this as it is too late. Do it earlier in software. > > ip_fragement() has such code, but in IPv6 it is missing. Note that > in6_proto_cksum_out() has to be called before the next protocol is > set to IPPROTO_FRAGMENT. > > ok? > > bluhm > > Index: netinet6/ip6_output.c > === > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_output.c,v > retrieving revision 1.269 > diff -u -p -r1.269 ip6_output.c > --- netinet6/ip6_output.c 29 Jun 2022 22:45:24 - 1.269 > +++ netinet6/ip6_output.c 6 Aug 2022 22:21:04 - > @@ -729,6 +729,12 @@ reroute: > mtu = IPV6_MAXPACKET; > > /* > + * If we are doing fragmentation, we can't defer TCP/UDP > + * checksumming; compute the checksum and clear the flag. > + */ > +in6_proto_cksum_out(m, NULL); > + > + /* >* Change the next header field of the last header in the >* unfragmentable part. >*/ > tested this using udp on em (with and without my offloading diff), igc, and ix openbsd-linux and linux-openbsd-linux-forwarding. ok mbuhl
Re: net*: Add *toc*() helpers to const-convert between AF specific and generic structs
Klemens Nanni wrote: > On Fri, Aug 05, 2022 at 05:08:14PM +0200, Claudio Jeker wrote: > > We added the inline functions for these typecasts to add a minimal level > > of error protection. Now where do we hit const problems? I have not seen > > such issues and I wonder if those just come from overzealous use of const > > arguments to functions. > > const problems quickly appear if we constify function arguments, which > does not strike me as overzealous when it documents the functions > behaviour, helps spotting side effects should for example future changes > suddenly make it write to pointer arguments, etc. But how often does that actually happen, so is it really that hugely valuable, compared to the expense of bizzare abstration in a .h file somewhere, which increases the knowlege required to work in this area, thereby also decreasing the developers who will work in the area? If you drive them down to zero, there will be no future changes you are scared of...
Re: net*: Add *toc*() helpers to const-convert between AF specific and generic structs
On Fri, Aug 05, 2022 at 05:08:14PM +0200, Claudio Jeker wrote: > We added the inline functions for these typecasts to add a minimal level > of error protection. Now where do we hit const problems? I have not seen > such issues and I wonder if those just come from overzealous use of const > arguments to functions. const problems quickly appear if we constify function arguments, which does not strike me as overzealous when it documents the functions behaviour, helps spotting side effects should for example future changes suddenly make it write to pointer arguments, etc.
Re: fgetln->getline in games/* (Was: Re: quiz(6): fully switch to getline(3))
Looks fine to me, though the free(answer) before exit(0) in quiz.c is not needed. OK millert@ - todd
What is ieee80211com->ic_ibss_chan?
Hi all, I am trying to understand the different channels in `struct ieee80211com`. I see 2 types of ieee80211_channel values: * ic_ibss_chan * ic_des_chan - Desired channel, pretty straight forward What is ic_ibss_chan used for? My initial guess was that it was the "current channel", but given that other implementations of net80211 (NetBSD, FreeBSD, Illumos) have `ic_curchan`, it does not seem to be the current channel. What is ic_ibss_channel used for? And where is the current channel information stored? Thanks! -- Farhan Khan PGP Fingerprint: 1312 89CE 663E 1EB2 179C 1C83 C41D 2281 F8DA C0DE
Re: rpki-client: add connect() MAX_CONTIMEOUT for rsync/rrdp
On Tue, Aug 02, 2022 at 12:27:57PM -0600, Theo de Raadt wrote: > I think you intend for that to be two seperate diffs, not merged into > one. > > For connect < 15 seconds, I think that is a bit strict. > > For IO stalling 15 seconds, I suspect such IO stalls happen more than > we know, and will do harm to RPKI processing results. > > I don't see any way this can be tested in less than 24 hours. Over the course of a couple of days hours I ran multiple instances (with the previously posted 15-second-timeout patch, and without) in a 'while true' loop. I also ran instances at 1-hour-cadance on slow machines. I've not been able to discern any harm to RPKI processing results. The big upside to the 15-second-timeout appears to be that total execution time is a fair chunk shorter. I'd like us to move forward with shorter connect() timeouts. Below is the changeset for just rsync. OK? Kind regards, Job Index: rsync.c === RCS file: /cvs/src/usr.sbin/rpki-client/rsync.c,v retrieving revision 1.38 diff -u -p -r1.38 rsync.c --- rsync.c 24 May 2022 09:20:49 - 1.38 +++ rsync.c 8 Aug 2022 12:57:18 - @@ -312,6 +312,7 @@ proc_rsync(char *prog, char *bind_addr, args[i++] = "-rt"; args[i++] = "--no-motd"; args[i++] = "--max-size=" STRINGIFY(MAX_FILE_SIZE); + args[i++] = "--contimeout=" STRINGIFY(MAX_CONTIMEOUT); args[i++] = "--timeout=180"; args[i++] = "--include=*/"; args[i++] = "--include=*.cer"; Index: extern.h === RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v retrieving revision 1.143 diff -u -p -r1.143 extern.h --- extern.h27 Jun 2022 10:18:27 - 1.143 +++ extern.h8 Aug 2022 12:57:18 - @@ -727,6 +727,9 @@ int mkpathat(int, const char *); #define MAX_HTTP_REQUESTS 64 #define MAX_RSYNC_REQUESTS 16 +/* How many seconds to wait for a connection to succeed. */ +#define MAX_CONTIMEOUT 15 + /* Maximum allowd repositories per tal */ #define MAX_REPO_PER_TAL 1000
Re: Split (*pr_usrreq)() by multiple handlers
On Mon, Aug 08, 2022 at 12:47:07AM +0200, Alexander Bluhm wrote:
> On Mon, Aug 08, 2022 at 01:03:13AM +0300, Vitaliy Makkoveev wrote:
> > > I prefer the first idiom. If there is an error, do something. We
> > > should not change the style in opposite direction. This will prevent
> > > consistency.
> > >
> >
> > I???m not entirely understand you. When we have something to do in the
> > error path, I use "goto out??? and it works like the first idiom.
>
> Ah, sorry. I was speaking about the != 0. The break, return, goto
> is fine. error != 0 is just style and does not affect correctness.
>
> > >> -if (error)
> > >> +if (error != 0)
>
We use if (error != 0) or if (m == NULL) idioms in the network stack, so
I used them too in the trivial places.
But here is another version of the split diff.
guenther@ reminded me about his reverted diff. His diff introduced the
new 'pr_usrreqs' structure, which should contain pointers to user
request handlers. The diff was reverted because it had a bug in the
socreate() path.
The bug was easy to fix, we need to check "prp->pr_usrreqs == NULL"
instead of "prp->pr_attach == NULL" because `pr_attach' handler was
moved to the 'pr_usrreqs' structure. guenther@ nas no resources to
continue this work, so he proposed to me to continue.
I still like to use dedicated 'pr_usrreqs' structure instead of place
pointers to `protosw' because it allows us to avoid huge copy-paste
within in{,6}_proto.c.
The diff below uses original idea, but it moves existing `pr_attach',
`pr_detach' and `pr_usrreq' to the new 'pr_usrreqs' structure. The
original diff moved only `pr_attach' and `pr_detach' and left
`pr_usrreq' as is. The original diff also introduced the `pru_control'
handler, but I like to start split with the following diffs.
Also I like to hide ugly "(*so->so_proto->pr_usrreqs->pru_usrreq)" calls
within corresponding pru_*() wrappers.
So, for the first step, just move existing user requests handlers
pointers.
Index: sys/kern/sys_socket.c
===
RCS file: /cvs/src/sys/kern/sys_socket.c,v
retrieving revision 1.51
diff -u -p -r1.51 sys_socket.c
--- sys/kern/sys_socket.c 20 Jun 2022 01:39:44 - 1.51
+++ sys/kern/sys_socket.c 8 Aug 2022 12:42:41 -
@@ -138,8 +138,7 @@ soo_ioctl(struct file *fp, u_long cmd, c
if (IOCGROUP(cmd) == 'r')
return (EOPNOTSUPP);
KERNEL_LOCK();
- error = ((*so->so_proto->pr_usrreq)(so, PRU_CONTROL,
- (struct mbuf *)cmd, (struct mbuf *)data, NULL, p));
+ error = pru_control(so, cmd, data, NULL, p);
KERNEL_UNLOCK();
break;
}
@@ -161,8 +160,7 @@ soo_stat(struct file *fp, struct stat *u
ub->st_mode |= S_IWUSR | S_IWGRP | S_IWOTH;
ub->st_uid = so->so_euid;
ub->st_gid = so->so_egid;
- (void) ((*so->so_proto->pr_usrreq)(so, PRU_SENSE,
- (struct mbuf *)ub, NULL, NULL, p));
+ (void)pru_sense(so, ub);
sounlock(so);
return (0);
}
Index: sys/kern/uipc_proto.c
===
RCS file: /cvs/src/sys/kern/uipc_proto.c,v
retrieving revision 1.22
diff -u -p -r1.22 uipc_proto.c
--- sys/kern/uipc_proto.c 25 Feb 2022 23:51:03 - 1.22
+++ sys/kern/uipc_proto.c 8 Aug 2022 12:42:41 -
@@ -50,27 +50,21 @@ const struct protosw unixsw[] = {
.pr_domain = &unixdomain,
.pr_protocol = PF_UNIX,
.pr_flags= PR_CONNREQUIRED|PR_WANTRCVD|PR_RIGHTS,
- .pr_usrreq = uipc_usrreq,
- .pr_attach = uipc_attach,
- .pr_detach = uipc_detach,
+ .pr_usrreqs = &uipc_usrreqs,
},
{
.pr_type = SOCK_SEQPACKET,
.pr_domain = &unixdomain,
.pr_protocol = PF_UNIX,
.pr_flags= PR_ATOMIC|PR_CONNREQUIRED|PR_WANTRCVD|PR_RIGHTS,
- .pr_usrreq = uipc_usrreq,
- .pr_attach = uipc_attach,
- .pr_detach = uipc_detach,
+ .pr_usrreqs = &uipc_usrreqs,
},
{
.pr_type = SOCK_DGRAM,
.pr_domain = &unixdomain,
.pr_protocol = PF_UNIX,
.pr_flags= PR_ATOMIC|PR_ADDR|PR_RIGHTS,
- .pr_usrreq = uipc_usrreq,
- .pr_attach = uipc_attach,
- .pr_detach = uipc_detach,
+ .pr_usrreqs = &uipc_usrreqs,
}
};
Index: sys/kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.280
diff -u -p -r1.280 uipc_socket.c
--- sys/kern/uipc_socket.c 25 Jul 2022 07:28:22 - 1.280
+++ sys/kern/uipc_socket.c 8 Aug 2022 12:42:41 -
@@ -172,7 +172,7 @@ socreate(int dom, struct socket **aso, i
prp = pffindproto(dom, proto, type);
else
prp = pffindtype(dom, type);
- if (prp == NULL || prp->pr_attach == NULL)
+ if (prp == NULL || prp->pr_usrreqs == NULL)
return (EPROTONOSUPPORT);
if (prp->pr_type != t
Re: [External] : inpcb lookup ref counting
Hello, diff looks good to me as far as I can tell. OK sashan@
pfctl: fix POM_STICKYADDRESS check
Hi,
This issue was reported to FreeBSD by Franco Fichtner
and is relevant to OpenBSD as well.
The POM_STICKYADDRESS flag is only ever set in pool_opts, but was
checked in filter_opts. That’s clearly not intended.
The parser tries to prevent "sticky-address sticky-address" syntax but
was actually cross-rule enforcing that ICMP filter cannot be before the
use of "sticky-address" in next rule.
E.g.
pass inet proto icmp icmp-type {unreach}
pass in route-to (if0 127.0.0.1/8) sticky-address inet
patch:
diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index 8a92a7e895c..ab8222f9e95 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -3684,7 +3684,7 @@ pool_opt : BITMASK {
pool_opts.staticport = 1;
}
| STICKYADDRESS {
- if (filter_opts.marker & POM_STICKYADDRESS) {
+ if (pool_opts.marker & POM_STICKYADDRESS) {
yyerror("sticky-address cannot be
redefined");
YYERROR;
}
Kristof
Re: mips64: trigger deferred timer interrupt from splx(9)
On Sun, Aug 07, 2022 at 11:05:37AM +, Visa Hankala wrote:
> On Sun, Jul 31, 2022 at 01:28:18PM -0500, Scott Cheloha wrote:
> > Apparently mips64, i.e. octeon and loongson, has the same problem as
> > powerpc/macppc and powerpc64. The timer interrupt is normally only
> > logically masked, not physically masked in the hardware, when we're
> > running at or above IPL_CLOCK. If we arrive at cp0_int5() when the
> > clock interrupt is logically masked we postpone all work until the
> > next tick. This is a problem for my WIP clock interrupt work.
>
> I think the use of logical masking has been a design choice, not
> something dictated by the hardware. Physical masking should be possible,
> but some extra care would be needed to implement it, as the mips64
> interrupt code is a bit clunky.
That would be cleaner, but from the sound of it, it's easier to start
with this.
> > So, this patch is basically the same as what I did for macppc and what
> > I have proposed for powerpc64.
> >
> > - Add a new member, ci_timer_deferred, to mips64's cpu_info struct.
> >
> > While here, remove ci_pendingticks. We don't need it anymore.
> >
> > - If we get to cp0_int5() and our IPL is too high, set
> > cpu_info.ci_timer_deferred and return.
> >
> > - If we get to cp0_int5() and our IPL is low enough, clear
> > cpu_info.ci_timer_deferred and do clock interrupt work.
> >
> > - In splx(9), if the new IPL is low enough and cpu_info.ci_timer_deferred
> > is set, trigger the clock interrupt.
> >
> > The only big difference is that mips64 uses an equality comparison
> > when deciding whether to arm the timer interrupt, so it's really easy
> > to "miss" CP0.count when you're setting CP0.compare.
> >
> > To address this I've written a function, cp0_raise_int5(), that spins
> > until it is sure the timer interrupt will go off. The function needed
> > a bit of new code for reading CP0.cause, which I've added to
> > cp0access.S. I am using an initial offset of 16 cycles based on
> > experimentation with the machine I have access to, a 500Mhz CN50xx.
> > Values lower than 16 require more than one loop to arm the timer. If
> > that value is insufficient for other machines we can try passing the
> > initial offset as an argument to the function.
>
> It should not be necessary to make the initial offset variable. The
> offset is primarily a function of the length and content of the
> instruction sequence. Some unpredictability comes from cache misses
> and maybe branch prediction failures.
Gotcha. So it mostly depends on the number of instructions between
loading CP0.count and storing CP0.compare.
> > I wasn't sure where to put the prototype for cp0_raise_int5() so I
> > stuck it in mips64/cpu.h. If there's a better place for it, just say
> > so.
>
> Currently, mips64 clock.c is formulated as a proper driver. I think
> callers should not invoke its functions directly but use a hook instead.
> The MI mips64 code starts the clock through the md_startclock function
> pointer. Maybe there could be md_triggerclock.
>
> To reduce risk of confusion, I would rename cp0_raise_int5 to
> cp0_trigger_int5, as `raise' overlaps with the spl API. Also,
> ci_clock_deferred instead of ci_timer_deferred would look more
> consistent with the surrounding code.
Okay, I took all these suggestions and incorporated them. Updated
patch attached.
One thing I'm still uncertain about is how glxclk fits into the
loongson picture. It's an interrupt clock that runs hardclock() and
statclock(), but the code doesn't do any logical masking, so I don't
know whether or not I need to adjust anything in that code or account
for it at all. If there's no logical masking there's no deferral, so
it would never call need to call md_triggerclock() from splx(9).
Also:
My EdgeRouter PoE just finished a serial `make build`. Took almost 12
days. Which is a good sign! Lots of opportunity for the patch to
fail and the clock to die.
In that time, under what I assume is relatively heavy load, the clock
interrupt deferral counters look like this:
cp0_raise_calls at 0x81701308: 133049
cp0_raise_miss at 0x81701300: 0
So 16 cycles as the initial offset works great. We never ran the loop
more than once, i.e. we never "missed" CP0.count.
The machine has been up a little more than a million seconds. So, at
100hz, with no separate statclock, and 2 CPUs, we'd expect ~200 clock
interrupts a second, or 200 million in total.
In ~200,000,000 cp0_int5() calls, we deferred ~133,000 of them, or
~0.0665%.
Index: mips64/mips64/clock.c
===
RCS file: /cvs/src/sys/arch/mips64/mips64/clock.c,v
retrieving revision 1.45
diff -u -p -r1.45 clock.c
--- mips64/mips64/clock.c 6 Apr 2022 18:59:26 - 1.45
+++ mips64/mips64/clock.c 8 Aug 2022 07:41:44 -
@@ -60,6 +60,7 @@ const struct cfattach clock_ca = {
};
void cp0_startclock(struct cpu_info *);
+void cp0_trigge
Re: top(1): display uptime in HH:MM:SS format
OK bluhm@
On Sun, Aug 07, 2022 at 06:22:38PM -0500, Scott Cheloha wrote:
> On Fri, Sep 18, 2020 at 03:59:05PM -0500, Scott Cheloha wrote:
> >
> > [...]
> >
> > - An HH:MM:SS format uptime is useful in top(1). It's also more
> > visually consistent with the local timestamp printed on the line
> > above it, so it is easier to read at a glance.
> >
> > - The variable printing of "days" is annoying. I would rather it
> > just told me "0 days" if it had been less than one day. It sucks
> > when the information you want moves around or isn't shown at all.
> > It's clever, sure, but I'd rather it be consistent.
> >
> > This patch changes the uptime format string to "up D days HH:MM:SS".
> > The format string does not vary with the elapsed uptime. There is no
> > inclusion/omission of the plural suffix depending on whether days is
> > equal to one.
> >
> > [...]
>
> Whoops, forgot about this one. September 18, 2020. What a time to be
> alive.
>
> Let's try this again. 98 week bump.
>
> To recap, this patch makes the uptime formatting in top(1) produce
> more constant-width results. The formatting is now always:
>
> up D days HH:MM:SS
>
> so only the day-count changes size. The day-count is also always
> printed: if the machine has not been up for a full day it prints
>
> up 0 days HH:MM:SS
>
> For example, the upper lines on the top(1) running on my machine
> currently look like this:
>
> load averages: 0.29, 0.29, 0.27 jetsam.attlocal.net
> 18:12:16
> 82 processes: 81 idle, 1 on processorup 3 days
> 07:14:01
>
> I have been running with this for almost two years and I love it.
> I would like to commit it.
>
> The only feedback I got when I originally posted this was that the
> output formatting would no longer be the same as uptime(1)'s. I don't
> think that matters very much. The person who offered the feedback
> didn't think it mattered either, they were just hypothesizing
> objections.
>
> ok?
>
> Index: display.c
> ===
> RCS file: /cvs/src/usr.bin/top/display.c,v
> retrieving revision 1.65
> diff -u -p -r1.65 display.c
> --- display.c 26 Aug 2020 16:21:28 - 1.65
> +++ display.c 7 Aug 2022 23:14:25 -
> @@ -208,31 +208,28 @@ display_init(struct statics * statics)
> return (display_lines);
> }
>
> +/*
> + * Print the time elapsed since the system booted.
> + */
> static void
> format_uptime(char *buf, size_t buflen)
> {
> - time_t uptime;
> - int days, hrs, mins;
> struct timespec boottime;
> + time_t uptime;
> + unsigned int days, hrs, mins, secs;
> +
> + if (clock_gettime(CLOCK_BOOTTIME, &boottime) == -1)
> + err(1, "clock_gettime");
>
> - /*
> - * Print how long system has been up.
> - */
> - if (clock_gettime(CLOCK_BOOTTIME, &boottime) != -1) {
> - uptime = boottime.tv_sec;
> - uptime += 30;
> - days = uptime / (3600 * 24);
> - uptime %= (3600 * 24);
> - hrs = uptime / 3600;
> - uptime %= 3600;
> - mins = uptime / 60;
> - if (days > 0)
> - snprintf(buf, buflen, "up %d day%s, %2d:%02d",
> - days, days > 1 ? "s" : "", hrs, mins);
> - else
> - snprintf(buf, buflen, "up %2d:%02d",
> - hrs, mins);
> - }
> + uptime = boottime.tv_sec;
> + days = uptime / (3600 * 24);
> + uptime %= (3600 * 24);
> + hrs = uptime / 3600;
> + uptime %= 3600;
> + mins = uptime / 60;
> + secs = uptime % 60;
> + snprintf(buf, buflen, "up %u days %02u:%02u:%02u",
> + days, hrs, mins, secs);
> }
>
>
fgetln->getline in games/* (Was: Re: quiz(6): fully switch to getline(3))
bump
Omar Polo wrote:
> Ben Fuller wrote:
> > On Sun, Jul 03, 2022 at 02:31:42 +0100, Ben Fuller wrote:
> > > One of the fgetln(3)s was previously converted to getline(3), but not
> > > the other.
> > >
> > > diff --git games/quiz/quiz.c games/quiz/quiz.c
> > > index d4fd0604e0d..362fb1e24a8 100644
> > > --- games/quiz/quiz.c
> > > +++ games/quiz/quiz.c
> > > @@ -224,7 +224,8 @@ quiz(void)
> > > {
> > > QE *qp;
> > > int i;
> > > - size_t len;
> > > + size_t size;
> > > + ssize_t len;
> > > u_int guesses, rights, wrongs;
> > > int next;
> > > char *answer, *t, question[LINE_SZ];
> > > @@ -275,12 +276,15 @@ quiz(void)
> > > qp->q_answered = TRUE;
> > > continue;
> > > }
> > > + size = 0;
> > > + answer = NULL;
>
> here you're leaking memory.
>
> This whole block is in a loop so you're possibly overriding the memory
> allocated in a previous cycle. size and answer should be initialized
> outside of the loop.
>
> otherwise the diff reads fine to me and game works fine.
>
> > > qp->q_asked = TRUE;
> > > (void)printf("%s?\n", question);
> > > for (;; ++guesses) {
> > > - if ((answer = fgetln(stdin, &len)) == NULL ||
> > > + if ((len = getline(&answer, &size, stdin)) == -1 ||
> > > answer[len - 1] != '\n') {
> > > score(rights, wrongs, guesses);
> > > + free(answer);
> > > exit(0);
> > > [...]
>
> however, while limiting ourselves to replace fgetln with getline in
> quiz(6) only? other games could take advantage of that :)
>
> (i couldn't help myself not to tweak save_file_name in battlestar too)
>
> briefly (play)tested the affected games, everything seems fine.
>
> ok?
Index: battlestar/cypher.c
===
RCS file: /home/cvs/src/games/battlestar/cypher.c,v
retrieving revision 1.20
diff -u -p -r1.20 cypher.c
--- battlestar/cypher.c 9 May 2019 20:19:22 - 1.20
+++ battlestar/cypher.c 8 Aug 2022 07:05:43 -
@@ -84,8 +84,9 @@ cypher(void)
int n;
int junk;
int lflag = -1;
- char *filename, *rfilename;
- size_t filename_len;
+ char *line = NULL, *filename;
+ size_t linesize = 0;
+ ssize_t linelen;
while (wordnumber <= wordcount) {
if (wordtype[wordnumber] != VERB &&
@@ -357,19 +358,16 @@ cypher(void)
case SAVE:
printf("\nSave file name (default %s): ",
DEFAULT_SAVE_FILE);
- filename = fgetln(stdin, &filename_len);
- if (filename_len == 0
- || (filename_len == 1 && filename[0] == '\n'))
- rfilename = save_file_name(DEFAULT_SAVE_FILE,
- strlen(DEFAULT_SAVE_FILE));
+ linelen = getline(&line, &linesize, stdin);
+ if (linelen == -1 || *line == '\n')
+ filename = save_file_name(DEFAULT_SAVE_FILE);
else {
- if (filename[filename_len - 1] == '\n')
- filename_len--;
- rfilename = save_file_name(filename,
- filename_len);
+ if (line[linelen - 1] == '\n')
+ line[linelen - 1] = '\0';
+ filename = save_file_name(line);
}
- save(rfilename);
- free(rfilename);
+ save(filename);
+ free(filename);
break;
case VERBOSE:
@@ -463,6 +461,9 @@ cypher(void)
}
if (!lflag)
newlocation();
+
+ free(line);
+
if (wordnumber < wordcount && !stop_cypher &&
(*words[wordnumber] == ',' || *words[wordnumber] == '.')) {
wordnumber++;
Index: battlestar/extern.h
===
RCS file: /home/cvs/src/games/battlestar/extern.h,v
retrieving revision 1.22
diff -u -p -r1.22 extern.h
--- battlestar/extern.h 5 Feb 2022 23:00:20 - 1.22
+++ battlestar/extern.h 8 Aug 2022 07:05:43 -
@@ -357,7 +357,7 @@ void ravage(void);
void restore(const char *);
int ride(void);
void save(const char *);
-char *save_file_name(const char *, size_t);
+char *save_file_name(const char *);
int shoot(void);
int take(unsigned int[]);
int takeoff(void);
Index: battlestar/init.c
===
RCS file: /home/cvs/src/games/battlestar/init.c,v
retrieving rev
