Re: patterns.c question or possible bug
On Tue, Jan 30, 2018 at 07:48:17AM +0100, Otto Moerbeek wrote:
> On Mon, Jan 29, 2018 at 11:23:18PM -0600, Edgar Pettijohn wrote:
>
> > I'm trying to use patterns.c for some pattern matching. The manual mentions
> > captures using "()" around what you want to capture. I don't see how to get
> > at the data though. Here is a sample program.
> >
> > #include
> > #include "patterns.h"
> >
> > int
> > main(int argc, char *argv[])
> > {
> > const char*errstr = NULL;
> > const char*string = "the quick the brown the fox";
> > const char*pattern = "the";
> > intret;
> > struct str_match match;
> >
> > ret = str_match(string, pattern, &match, &errstr);
> >
> > if (errstr != NULL)
> > printf("%s\n", errstr);
> > else
> > printf("number of matches %d\n", match.sm_nmatch);
> >
> > return 0;
> > }
> >
> > It prints 2 which I was expecting 3. I've tried multiple other patterns and
> > it seems the answer is always 2. Which leads me to believe I'm doing
> > something wrong. Any assistance appreciated.
> >
> >
> > Thanks,
> >
> >
> > Edgar
>
> Hmm, str_match() isn't a function in any OpenBSD API. So I have no
> idea what function you are talking about.
>
> -Otto
>
It is in httpd patterns.c, which is based on the LUA pattern matching code.
--
Kind regards,
Hiltjo
Re: typo hcreate.3
On Mon, Jan 29, 2018 at 05:47:14PM -0600, Edgar Pettijohn wrote: > Index: hcreate.3 > == > RCS file: /cvs/src/lib/libc/stdlib/hcreate.3,v > retrieving revision 1.7 > diff -u -p -u -r1.7 hcreate.3 > --- hcreate.3 12 Mar 2016 21:31:22 - 1.7 > +++ hcreate.3 29 Jan 2018 23:45:08 - > @@ -78,7 +78,7 @@ the data can no longer be accessed. > .Pp > The > .Fn hsearch > -function is used to search to the hash table. > +function is used to search the hash table. > It returns a pointer into the > hash table indicating the address of an item. > The > > > I contemplated changing the second `to' to `through', but less seems to > be more. > fixed, thanks. jmc
Re: patterns.c question or possible bug
On Jan 30, 2018 12:05 AM, Ori Bernstein wrote:
>
> On Mon, 29 Jan 2018 23:23:18 -0600, Edgar Pettijohn
> wrote:
>
> > I'm trying to use patterns.c for some pattern matching. The manual
> > mentions captures using "()" around what you want to capture. I don't
> > see how to get at the data though. Here is a sample program.
> >
> > #include
> > #include "patterns.h"
> >
> > int
> > main(int argc, char *argv[])
> > {
> > const char *errstr = NULL;
> > const char *string = "the quick the brown the fox";
> > const char *pattern = "the";
> > int ret;
> > struct str_match match;
> >
> > ret = str_match(string, pattern, &match, &errstr);
> >
> > if (errstr != NULL)
> > printf("%s\n", errstr);
> > else
> > printf("number of matches %d\n", match.sm_nmatch);
> >
> > return 0;
> > }
> >
> > It prints 2 which I was expecting 3. I've tried multiple other patterns
> > and it seems the answer is always 2. Which leads me to believe I'm doing
> > something wrong. Any assistance appreciated.
> >
> >
> > Thanks,
> >
> >
> > Edgar
>
> The code is looking for a match of the pattern in the string, not all matches
> of the pattern in the string. It also makes the (IMO, surprising) decision
> that not having any capture groups in the pattern implies capturing the whole
> pattern. The whole string goes into the first match.
>
> So, in your case, you're matching:
>
> "the quick the brown the fox";
> ^^^
>
> Accordingly:
>
> matches.sm_match[0] = "the quick the brown the fox"
> matches.sm_match[1] = "the"
>
> If you had 'quick', you'd get similar behavior:
>
> "the quick the brown the fox";
>
>
> Equivalently, putting the whole pattern in '()' will match the same thing:
>
> pattern = "(quick)"
>
> But multiple parens will match their substrings:
>
> pattern = "(qu)ick (the)"
>
> "the quick the brown the fox";
> ^^ ^^^
> matches.sm_match[0] = "the quick the brown the fox"
> matches.sm_match[1] = "qu"
> matches.sm_match[2] = "the"
>
> The choice to capture implicitly, I think, is confusing, but the behavior
> seems to me to be correct.
>
> --
> Ori Bernstein
Thanks. Makes sense now. Probably would have figured it out for myself if I'd
have printed out matches.sm_match[0], etc. Live and learn.
Edgar
Re: efiboot: fix TFTP reading of 0-size files
On Fri, Jan 26, 2018 at 12:25:56AM +0100, Christian Weisgerber wrote:
> Fix TFTP reading of zero-size files in efiboot:
>
> The AllocatePages EFI call returns an error when the allocation
> size is 0. Skip allocating memory and actually transferring the
> file when it is empty.
>
> Properly return the number of unread bytes so that a read() of n
> bytes does not return n if no bytes were read.
>
> While here, disallow lseek() beyond the TFTP file buffer for SEEK_CUR
> as we already do for SEEK_SET.
>
>
>
> Tested on aarch64. Can somebody test this on amd64?
> OK?
Works for me on amd64.
ok patrick@
>
> Index: arch/amd64/stand/efiboot/conf.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/conf.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 conf.c
> --- arch/amd64/stand/efiboot/conf.c 25 Nov 2017 19:02:07 - 1.12
> +++ arch/amd64/stand/efiboot/conf.c 25 Jan 2018 22:39:37 -
> @@ -39,7 +39,7 @@
> #include "efidev.h"
> #include "efipxe.h"
>
> -const char version[] = "3.36";
> +const char version[] = "3.37";
>
> #ifdef EFI_DEBUG
> int debug = 0;
> Index: arch/amd64/stand/efiboot/efipxe.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efipxe.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 efipxe.c
> --- arch/amd64/stand/efiboot/efipxe.c 21 Jan 2018 21:37:01 - 1.2
> +++ arch/amd64/stand/efiboot/efipxe.c 25 Jan 2018 22:46:15 -
> @@ -143,6 +143,9 @@ tftp_open(char *path, struct open_file *
> }
> tftpfile->inbufsize = size;
>
> + if (tftpfile->inbufsize == 0)
> + goto out;
> +
> status = EFI_CALL(BS->AllocatePages, AllocateAnyPages, EfiLoaderData,
> EFI_SIZE_TO_PAGES(tftpfile->inbufsize), &addr);
> if (status != EFI_SUCCESS) {
> @@ -157,7 +160,7 @@ tftp_open(char *path, struct open_file *
> free(tftpfile, sizeof(*tftpfile));
> return ENXIO;
> }
> -
> +out:
> f->f_fsdata = tftpfile;
> return 0;
> }
> @@ -167,8 +170,9 @@ tftp_close(struct open_file *f)
> {
> struct tftp_handle *tftpfile = f->f_fsdata;
>
> - EFI_CALL(BS->FreePages, (paddr_t)tftpfile->inbuf,
> - EFI_SIZE_TO_PAGES(tftpfile->inbufsize));
> + if (tftpfile->inbuf != NULL)
> + EFI_CALL(BS->FreePages, (paddr_t)tftpfile->inbuf,
> + EFI_SIZE_TO_PAGES(tftpfile->inbufsize));
> free(tftpfile, sizeof(*tftpfile));
> return 0;
> }
> @@ -184,15 +188,11 @@ tftp_read(struct open_file *f, void *add
> else
> toread = size;
>
> - if (toread == 0) {
> - if (resid != NULL)
> - *resid = 0;
> - return (0);
> + if (toread != 0) {
> + memcpy(addr, tftpfile->inbuf + tftpfile->inbufoff, toread);
> + tftpfile->inbufoff += toread;
> }
>
> - memcpy(addr, tftpfile->inbuf + tftpfile->inbufoff, toread);
> - tftpfile->inbufoff += toread;
> -
> if (resid != NULL)
> *resid = size - toread;
> return 0;
> @@ -211,7 +211,8 @@ tftp_seek(struct open_file *f, off_t off
>
> switch(where) {
> case SEEK_CUR:
> - if (tftpfile->inbufoff + offset < 0) {
> + if (tftpfile->inbufoff + offset < 0 ||
> + tftpfile->inbufoff + offset > tftpfile->inbufsize) {
> errno = EOFFSET;
> break;
> }
> Index: arch/arm64/stand/efiboot/conf.c
> ===
> RCS file: /cvs/src/sys/arch/arm64/stand/efiboot/conf.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 conf.c
> --- arch/arm64/stand/efiboot/conf.c 21 Jan 2018 21:35:34 - 1.10
> +++ arch/arm64/stand/efiboot/conf.c 25 Jan 2018 20:42:46 -
> @@ -36,7 +36,7 @@
> #include "efidev.h"
> #include "efipxe.h"
>
> -const char version[] = "0.9";
> +const char version[] = "0.10";
> int debug = 0;
>
> struct fs_ops file_system[] = {
> Index: arch/arm64/stand/efiboot/efipxe.c
> ===
> RCS file: /cvs/src/sys/arch/arm64/stand/efiboot/efipxe.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 efipxe.c
> --- arch/arm64/stand/efiboot/efipxe.c 21 Jan 2018 21:35:34 - 1.1
> +++ arch/arm64/stand/efiboot/efipxe.c 25 Jan 2018 22:45:08 -
> @@ -139,6 +139,9 @@ tftp_open(char *path, struct open_file *
> }
> tftpfile->inbufsize = size;
>
> + if (tftpfile->inbufsize == 0)
> + goto out;
> +
> status = EFI_CALL(BS->AllocatePages, AllocateAnyPages, EfiLoaderData,
> EFI_SIZE_TO_PAGES(tftpfile->inbufsize), &addr);
> if (status != EFI_SUCCESS) {
> @@ -153,7 +156,7 @@ tftp_open(char *path, struct open_file *
> free(tftpfile, sizeof(*tftpfile));
> return ENXIO;
>
Re: bridge(4): protected interface (port)
Hello Remi, Regarding your query, >Wouldn't this duplicate BUM traffic? In this situation I would try a setup with >trunk(4) for the uplinks. At least the failover mode should work if your >switches have a limited feature set. Yes the scenario where you put 2 uplinks into a bridge would potentially duplicate Broadcast, Unknown Unicast, & Multicast Traffic, as per a standard Layer2 switch, but you have the advantage, that if a port goes down and the mac is removed from the bridge Fib, the frame is briefly flooded until the mac is learned via the other layer2 path, so Unknown Unicast is the exception rather than the rule (thankfully) Broadcast / Multicast Traffic is usually small Conversely if you use a Trunk in fail over mode it will favor the primary interface until it goes down,so the primary would be used as-long as the interface is up even if the switch that primary is attached to has lost its up-link, All that being said the Feature that MPI (thanks for putting the time into) improves security and ease of configuration of Protected ports in a Bridge. and it in its self will actually Limit Broadcast / Multicast / unknown Unicast flooding in most cases (except for a Redundant Layer 2 Uplinks into the same Bridge) I Hope this helps Tom Smyth
pflow PF_OUT use WIRE ips
Hi,
A problem with our flows and nat-to on the $ext_if is that it exports the
original (private) IP address and not the new-public IP after the translation.
We already have the information about the private IP from the flow on the
$int_if.
Similar problem with rdr-to and PF_OUT.
This diff changes st->key to use PF_SK_WIRE for PF_OUT and export what you see
in tcpdump.
Tested with PF_IN/PF_OUT and normal, nat-to, rdr-to connections,
although there is problem only with PF_OUT which used PF_SK_STACK.
Did not test IPv6.
regards,
Giannis
ps. I'll make an attempt to add NEL extension record types to hold NAT
information in IPFIX
from https://tools.ietf.org/html/draft-ietf-behave-ipfix-nat-logging-13
nfdump already supports this info so it will be good to be able to export it.
Index: if_pflow.c
===
RCS file: /cvs/src/sys/net/if_pflow.c,v
retrieving revision 1.86
diff -u -p -r1.86 if_pflow.c
--- if_pflow.c 9 Jan 2018 15:24:24 - 1.86
+++ if_pflow.c 30 Jan 2018 13:10:46 -
@@ -786,7 +786,7 @@ export_pflow(struct pf_state *st)
struct pflow_softc *sc = NULL;
struct pf_state_key *sk;
- sk = st->key[st->direction == PF_IN ? PF_SK_WIRE : PF_SK_STACK];
+ sk = st->key[PF_SK_WIRE];
SLIST_FOREACH(sc, &pflowif_list, sc_next) {
switch (sc->sc_version) {
Re: init(8): check GETTY_SPACING against monotonic clock
On Mon, 29 Jan 2018 19:32:05 -0600, Scott Cheloha wrote: > This ensures that the GETTY_SPACING check remains useful if > the system clock is set backwards. > > The time comparison is meaningless unless the timespec is set, > so there's no reason to go to the kernel to get the time unless > we have something to look at. And unless I'm missing something > there isn't any reason to continue retrieving the current time > in the parent process if we're doing the comparison in the child > process, so I've moved the system call into that block. Looks good, OK millert@ - todd
Re: tcpdump(8) USB support
On 30/01/18(Tue) 16:37, David Gwynne wrote:
> > [...]
> > bpfsdetach takes the reference to a bpf interface that bpfsattach
> > returns. the manpage likely isnt clear. the diff below fixes it.
> >
> > ive also added a bpf_tap_hdr function that lets you pass a header
> > along with a flat buffer. internally it shoves them into mbufs, but
> > the caller doesnt have to know that. it also doesnt have to allocate
> > a temporary buffer to pass along.
> >
> > ive also made usb_tap return early if bpf hasnt set the bpf pointer
> > yet.
>
> oops, this diff was stale and had bufs in bpf_tap_hdr.
Thanks David that's exactly what I needed!
Here's an updated diff that:
- fixes big-endian support in tcpdump(8)'s USBPCAP parser
- emits SETUP/DATA/STATUS stages for control transfers, that makes
Wireshark's parser happy.
I haven't implemented isochronous yet, but bulk/intr/ctrl can now be
easily debugged with Wireshark. Improving our tcpdump(8) parser would
be the next step.
How should proceed, do you want to commit your bpf_tap_hdr(9) and the
related doc? That's obviously ok mpi@.
Index: sys/net/bpf.c
===
RCS file: /cvs/src/sys/net/bpf.c,v
retrieving revision 1.166
diff -u -p -r1.166 bpf.c
--- sys/net/bpf.c 24 Jan 2018 00:25:17 - 1.166
+++ sys/net/bpf.c 30 Jan 2018 12:35:59 -
@@ -1291,6 +1291,45 @@ _bpf_mtap(caddr_t arg, const struct mbuf
}
/*
+ * Incoming linkage from device drivers, where a data buffer should be
+ * prepended by an arbitrary header. In this situation we already have a
+ * way of representing a chain of memory buffers, ie, mbufs, so reuse
+ * the existing functionality by attaching the buffers to mbufs.
+ *
+ * Con up a minimal mbuf chain to pacify bpf by allocating (only) a
+ * struct m_hdr each for the header and data on the stack.
+ */
+int
+bpf_tap_hdr(caddr_t arg, const void *hdr, unsigned int hdrlen,
+const void *buf, unsigned int buflen, u_int direction)
+{
+ struct m_hdr mh, md;
+ struct mbuf *m0 = NULL;
+ struct mbuf **mp = &m0;
+
+ if (hdr != NULL) {
+ mh.mh_flags = 0;
+ mh.mh_next = NULL;
+ mh.mh_len = hdrlen;
+ mh.mh_data = (void *)hdr;
+
+ *mp = (struct mbuf *)&mh;
+ mp = &mh.mh_next;
+ }
+
+ if (buf != NULL) {
+ md.mh_flags = 0;
+ md.mh_next = NULL;
+ md.mh_len = buflen;
+ md.mh_data = (void *)buf;
+
+ *mp = (struct mbuf *)&md;
+ }
+
+ return _bpf_mtap(arg, m0, direction, bpf_mcopy);
+}
+
+/*
* Incoming linkage from device drivers, when packet is in an mbuf chain.
*/
int
Index: sys/net/bpf.h
===
RCS file: /cvs/src/sys/net/bpf.h,v
retrieving revision 1.63
diff -u -p -r1.63 bpf.h
--- sys/net/bpf.h 24 Jan 2018 00:25:17 - 1.63
+++ sys/net/bpf.h 30 Jan 2018 12:35:59 -
@@ -201,6 +201,7 @@ struct bpf_hdr {
#define DLT_USER13 160 /* Reserved for private use */
#define DLT_USER14 161 /* Reserved for private use */
#define DLT_USER15 162 /* Reserved for private use */
+#define DLT_USBPCAP249 /* USBPcap */
#define DLT_MPLS 219 /* MPLS Provider Edge header */
#define DLT_OPENFLOW 267 /* in-kernel OpenFlow, by pcap */
@@ -311,6 +312,7 @@ int bpf_mtap_hdr(caddr_t, caddr_t, u_in
void (*)(const void *, void *, size_t));
int bpf_mtap_af(caddr_t, u_int32_t, const struct mbuf *, u_int);
int bpf_mtap_ether(caddr_t, const struct mbuf *, u_int);
+int bpf_tap_hdr(caddr_t, const void *, u_int, const void *, u_int, u_int);
voidbpfattach(caddr_t *, struct ifnet *, u_int, u_int);
voidbpfdetach(struct ifnet *);
void *bpfsattach(caddr_t *, const char *, u_int, u_int);
Index: usr.sbin/tcpdump/Makefile
===
RCS file: /cvs/src/usr.sbin/tcpdump/Makefile,v
retrieving revision 1.62
diff -u -p -r1.62 Makefile
--- usr.sbin/tcpdump/Makefile 30 Oct 2017 10:07:44 - 1.62
+++ usr.sbin/tcpdump/Makefile 30 Jan 2018 12:35:59 -
@@ -49,7 +49,7 @@ SRCS= tcpdump.c addrtoname.c privsep.c p
print-etherip.c print-lwres.c print-lldp.c print-cdp.c print-pflog.c \
print-pfsync.c pf_print_state.c print-ofp.c ofp_map.c \
print-udpencap.c print-carp.c \
- print-802_11.c print-iapp.c print-mpls.c print-slow.c \
+ print-802_11.c print-iapp.c print-mpls.c print-slow.c print-usbpcap.c \
gmt2local.c savestr.c setsignal.c in_cksum.c
# TCP OS Fingerprinting
Index: usr.sbin/tcpdump/interface.h
===
RCS file: /cvs/src/usr.sbin/tcpdump/interface.h,v
retrieving revision 1.69
diff -u -p -r1.69 interface.h
--- usr.sbin/tcpdump/i
Re: arm64: recognize netboot
> Date: Mon, 29 Jan 2018 20:23:09 +0100
> From: Christian Weisgerber
>
> This allows an arm64 kernel to recognize that it has been netbooted
> and to add the boot interface to the "netboot" group. efiboot grabs
> the MAC address from the PXE environment, passes it to the kernel,
> where it is matched against the list of ethernet interfaces and the
> boot device is set. Concept and most of the code cribbed from amd64.
>
> Welcome to the OpenBSD/arm64 6.2 installation program.
> Starting non-interactive mode in 5 seconds...
> (I)nstall, (U)pgrade, (A)utoinstall or (S)hell?
>
> There are probably too many printfs in diskconf()...
>
> bootfile: tftp0a:/bsd
> PXE boot MAC address e0:ff:f7:00:20:3c, interface msk0
> boot device: msk0
>
> ... but I don't know what we want there. I'm also uncertain about
> the NFSCLIENT ifdef, but the x86 code evolved into this over a series
> of commits.
>
> I've omitted the efiboot version bump since that would conflict
> with my other efiboot diff.
The proper Open Firmware way of doing this is to privide a "bootpath"
variable under /chosen, but that won't work for the FDT since the PCI
network interface isn't present in the tree. So this approach is
prefectly fine with me.
ok kettenis@
> Index: arch/arm64/arm64/autoconf.c
> ===
> RCS file: /cvs/src/sys/arch/arm64/arm64/autoconf.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 autoconf.c
> --- arch/arm64/arm64/autoconf.c 27 Jan 2018 22:55:23 - 1.8
> +++ arch/arm64/arm64/autoconf.c 29 Jan 2018 17:53:28 -
> @@ -20,9 +20,15 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> +#include
> +#include
> +#include
> +#include
> +
> #include
>
> extern void dumpconf(void);
> @@ -63,6 +69,7 @@ diskconf(void)
> size_t len;
> char*p;
> dev_t tmpdev;
> + extern uint8_t *bootmac;
>
> if (*boot_file != '\0')
> printf("bootfile: %s\n", boot_file);
> @@ -75,6 +82,27 @@ diskconf(void)
> else
> len = strlen(boot_file);
> bootdv = parsedisk(boot_file, len, 0, &tmpdev);
> + }
> +
> + if (bootmac) {
> + struct ifnet *ifp;
> +
> + TAILQ_FOREACH(ifp, &ifnet, if_list) {
> + if (ifp->if_type == IFT_ETHER &&
> + memcmp(bootmac, ((struct arpcom *)ifp)->ac_enaddr,
> + ETHER_ADDR_LEN) == 0)
> + break;
> + }
> + if (ifp) {
> +#if defined(NFSCLIENT)
> + printf("PXE boot MAC address %s, interface %s\n",
> + ether_sprintf(bootmac), ifp->if_xname);
> + bootdv = parsedisk(ifp->if_xname, strlen(ifp->if_xname),
> + 0, &tmpdev);
> +#endif
> + } else
> + printf("PXE boot MAC address %s, interface %s\n",
> + ether_sprintf(bootmac), "unknown");
> }
>
> if (bootdv != NULL)
> Index: arch/arm64/arm64/machdep.c
> ===
> RCS file: /cvs/src/sys/arch/arm64/arm64/machdep.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 machdep.c
> --- arch/arm64/arm64/machdep.c28 Jan 2018 13:17:45 - 1.27
> +++ arch/arm64/arm64/machdep.c29 Jan 2018 16:38:32 -
> @@ -52,6 +52,8 @@
> char *boot_args = NULL;
> char *boot_file = "";
>
> +uint8_t *bootmac = NULL;
> +
> extern uint64_t esym;
>
> int stdout_node = 0;
> @@ -778,16 +780,23 @@ initarm(struct arm64_bootparams *abp)
>
> node = fdt_find_node("/chosen");
> if (node != NULL) {
> - char *args, *duid, *prop;
> + char *prop;
> int len;
> + static uint8_t lladdr[6];
>
> - len = fdt_node_property(node, "bootargs", &args);
> + len = fdt_node_property(node, "bootargs", &prop);
> if (len > 0)
> - collect_kernel_args(args);
> + collect_kernel_args(prop);
>
> - len = fdt_node_property(node, "openbsd,bootduid", &duid);
> + len = fdt_node_property(node, "openbsd,bootduid", &prop);
> if (len == sizeof(bootduid))
> - memcpy(bootduid, duid, sizeof(bootduid));
> + memcpy(bootduid, prop, sizeof(bootduid));
> +
> + len = fdt_node_property(node, "openbsd,bootmac", &prop);
> + if (len == sizeof(lladdr)) {
> + memcpy(lladdr, prop, sizeof(lladdr));
> + bootmac = lladdr;
> + }
>
> len = fdt_node_property(node, "openbsd,uefi-mmap-start", &prop);
> if (len == sizeof(mmap_start))
> Index: arch/arm64/stand/efiboot/efiboot.c
> ===
> RCS fi
Re: sleep(1): support longer naps
On Thu, Jan 25, 2018 at 09:35:33PM -0600, Scott Cheloha wrote:
> Hi,
>
> This increases the range for sleep(1) from 100 million seconds up to
> 9223372036854775807.
>
> POSIX.1-2001 added a stipulation (which POSIX.1-2008 retains) that sleep(1)
> support inputs of at least 2147483647 seconds. This patch fulfills that
> requirement and brings us into alignment with POSIX.1-2008, which we already
> claim anyway.
>
> FreeBSD supports up to INT_MAX and no further. GNU sleep keeps seconds
> in a double, so they too ought to support at least this minimum... but
> they do something weird that I haven't quite figured out yet, so I'm
> actually not sure what they support.
>
> But anyway I don't see a reason to cap user input (as FreeBSD does) at the
> positive 32-bit limit when we already carefully parse up to the limit of
> time_t and check for overflow. If the end user wants to sleep that long,
> we can just let them. I have a subsequent patch that tells the user that
> their input is too large in the event of overflow. Currently we just exit
> with EINVAL and say nothing, which is both unusual and unhelpful.
>
> Using a loop to extend the input range of the utility seems a bit unclean,
> but this case alone certainly doesn't warrant extending the input range for
> nanosleep(2).
>
> I'm not sure whether the larger range is actually an extension to the
> standard, and if so whether we need to mention it in the manpage, but
> I've included a diff here anyway just in case. If you have ideas about
> the wording I'd love to hear them.
>
> Thoughts? ok?
The diff for sleep.c is ok.
For the manpage part, I have no strong opinion, but I'm not so sure this
is needed/useful. That sleeping for fractions of a second isn't portable
definitely has practical relevance, sleeping longer than 68 years not so
much. I'll leave this for jmc to decide.
>
> --
> Scott Cheloha
>
> Index: bin/sleep/sleep.1
> ===
> RCS file: /cvs/src/bin/sleep/sleep.1,v
> retrieving revision 1.22
> diff -u -p -r1.22 sleep.1
> --- bin/sleep/sleep.1 16 Aug 2016 18:51:25 - 1.22
> +++ bin/sleep/sleep.1 26 Jan 2018 03:22:08 -
> @@ -108,8 +108,10 @@ utility is compliant with the
> .St -p1003.1-2008
> specification.
> .Pp
> -The handling of fractional arguments is provided as an extension to that
> -specification.
> +The handling of fractional arguments and the support for values of
> +.Ar seconds
> +greater than 2147483647
> +are extensions to that specification.
> .Sh HISTORY
> A
> .Nm
> Index: bin/sleep/sleep.c
> ===
> RCS file: /cvs/src/bin/sleep/sleep.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 sleep.c
> --- bin/sleep/sleep.c 11 Oct 2015 20:17:49 - 1.24
> +++ bin/sleep/sleep.c 26 Jan 2018 03:22:08 -
> @@ -102,12 +102,24 @@ main(int argc, char *argv[])
> }
> }
>
> - rqtp.tv_sec = secs;
> - rqtp.tv_nsec = nsecs;
> -
> - if ((secs > 0) || (nsecs > 0))
> + while (secs > 0 || nsecs > 0) {
> + /*
> + * nanosleep(2) supports a maximum of 100 million
> + * seconds, so we break the nap up into multiple
> + * calls if we have more than that.
> + */
> + if (secs > 1) {
> + rqtp.tv_sec = 1;
> + rqtp.tv_nsec = 0;
> + } else {
> + rqtp.tv_sec = secs;
> + rqtp.tv_nsec = nsecs;
> + }
> if (nanosleep(&rqtp, NULL))
> err(1, NULL);
> + secs -= rqtp.tv_sec;
> + nsecs -= rqtp.tv_nsec;
> + }
> return (0);
> }
>
>
Re: sleep(1): support longer naps
On Wed, Jan 31, 2018 at 05:52:35AM +0100, Theo Buehler wrote:
> On Thu, Jan 25, 2018 at 09:35:33PM -0600, Scott Cheloha wrote:
> > Hi,
> >
> > This increases the range for sleep(1) from 100 million seconds up to
> > 9223372036854775807.
> >
> > POSIX.1-2001 added a stipulation (which POSIX.1-2008 retains) that sleep(1)
> > support inputs of at least 2147483647 seconds. This patch fulfills that
> > requirement and brings us into alignment with POSIX.1-2008, which we already
> > claim anyway.
> >
> > FreeBSD supports up to INT_MAX and no further. GNU sleep keeps seconds
> > in a double, so they too ought to support at least this minimum... but
> > they do something weird that I haven't quite figured out yet, so I'm
> > actually not sure what they support.
> >
> > But anyway I don't see a reason to cap user input (as FreeBSD does) at the
> > positive 32-bit limit when we already carefully parse up to the limit of
> > time_t and check for overflow. If the end user wants to sleep that long,
> > we can just let them. I have a subsequent patch that tells the user that
> > their input is too large in the event of overflow. Currently we just exit
> > with EINVAL and say nothing, which is both unusual and unhelpful.
> >
> > Using a loop to extend the input range of the utility seems a bit unclean,
> > but this case alone certainly doesn't warrant extending the input range for
> > nanosleep(2).
> >
> > I'm not sure whether the larger range is actually an extension to the
> > standard, and if so whether we need to mention it in the manpage, but
> > I've included a diff here anyway just in case. If you have ideas about
> > the wording I'd love to hear them.
> >
> > Thoughts? ok?
>
> The diff for sleep.c is ok.
>
> For the manpage part, I have no strong opinion, but I'm not so sure this
> is needed/useful. That sleeping for fractions of a second isn't portable
> definitely has practical relevance, sleeping longer than 68 years not so
> much. I'll leave this for jmc to decide.
>
i tend to agree that it's not worth documenting, unless you have a
concrete reason to do so.
jmc
> >
> > --
> > Scott Cheloha
> >
> > Index: bin/sleep/sleep.1
> > ===
> > RCS file: /cvs/src/bin/sleep/sleep.1,v
> > retrieving revision 1.22
> > diff -u -p -r1.22 sleep.1
> > --- bin/sleep/sleep.1 16 Aug 2016 18:51:25 - 1.22
> > +++ bin/sleep/sleep.1 26 Jan 2018 03:22:08 -
> > @@ -108,8 +108,10 @@ utility is compliant with the
> > .St -p1003.1-2008
> > specification.
> > .Pp
> > -The handling of fractional arguments is provided as an extension to that
> > -specification.
> > +The handling of fractional arguments and the support for values of
> > +.Ar seconds
> > +greater than 2147483647
> > +are extensions to that specification.
> > .Sh HISTORY
> > A
> > .Nm
> > Index: bin/sleep/sleep.c
> > ===
> > RCS file: /cvs/src/bin/sleep/sleep.c,v
> > retrieving revision 1.24
> > diff -u -p -r1.24 sleep.c
> > --- bin/sleep/sleep.c 11 Oct 2015 20:17:49 - 1.24
> > +++ bin/sleep/sleep.c 26 Jan 2018 03:22:08 -
> > @@ -102,12 +102,24 @@ main(int argc, char *argv[])
> > }
> > }
> >
> > - rqtp.tv_sec = secs;
> > - rqtp.tv_nsec = nsecs;
> > -
> > - if ((secs > 0) || (nsecs > 0))
> > + while (secs > 0 || nsecs > 0) {
> > + /*
> > +* nanosleep(2) supports a maximum of 100 million
> > +* seconds, so we break the nap up into multiple
> > +* calls if we have more than that.
> > +*/
> > + if (secs > 1) {
> > + rqtp.tv_sec = 1;
> > + rqtp.tv_nsec = 0;
> > + } else {
> > + rqtp.tv_sec = secs;
> > + rqtp.tv_nsec = nsecs;
> > + }
> > if (nanosleep(&rqtp, NULL))
> > err(1, NULL);
> > + secs -= rqtp.tv_sec;
> > + nsecs -= rqtp.tv_nsec;
> > + }
> > return (0);
> > }
> >
> >
