Re: sbin/wsconsctl: show more data
On Sat, Jan 05, 2019 at 10:39:46PM +0200, li...@wrant.com wrote: > Just need a manual page and a tunable at some point, didn't much before. > Still needed more text lines on the console and utf8 options previously. > > Got less chars now, so console is a regression in data thus readability. > Also, mention of direction in change log / commit message would be nice. > > On 27" 2560x1440 panel, usable res was 1920x1200 160x54 and then 160x65. > Topic: show more data, now 160x45 (spleen 16x32) << 425x110 (misc 6x13). > > 2010-2020, stuck at about 50 lines at 100 PPI is a huge waste of pixels. > if you're going to jsut complain w/o a diff, you should do it at the source of it[0]; or just patch and move on. -Artturi [0]: https://bugzilla.kernel.org/show_bug.cgi?id=172421
Re: install(1) could fail due to race
Ted Unangst wrote: > Todd C. Miller wrote: > > On Mon, 07 Jan 2019 22:55:54 +, Christian Weisgerber wrote: > > > > > Oooh, that short period can be the time between disk flushes (30s?) > > > with softupdates. I think that's one of the scenarios where you > > > can run out of disk space if you have softupdates enabled. > > > > Yes, I think I've had that happen in the (now rather distant) past. > > Has anybody seen this recently? Many years ago I had a diff to fix it. Lost > the diff, but I remember it's not super complicated. I figured the problem > went away (due to less tight partitioning) and haven't seen it myself. If > people are still having to workaround it, I can cook something up. Wait. Stop. Some architectures triggered this "delayed release of softdep buffers" worse. In order of danger: macppc, hppa, sgi, ... To me, it seems to be related to "running servicing functions". I don't think this needs a workaround. I despise the word. The proper bug needs to be found, and fixed. Not some workaround installed which seems to fix the issue on machines people don't have, and golly gee it still remains or surfaces in some other way.. and then OH WE KNOW THE REAL PROBLEM, here is the real fix, and years later "someone forgot their broken workaround still remains in the tree causing other effects". Don't "workaround it". Workaround it by fixing it. I think macppc and hppa are teaching a lesson. deep rm -rf on hppa is super slow. On macppc, softdep is very slow at releasing buffers. Where are the MD mistakes to cause this?
Re: install(1) could fail due to race
Todd C. Miller wrote: > On Mon, 07 Jan 2019 22:55:54 +, Christian Weisgerber wrote: > > > Oooh, that short period can be the time between disk flushes (30s?) > > with softupdates. I think that's one of the scenarios where you > > can run out of disk space if you have softupdates enabled. > > Yes, I think I've had that happen in the (now rather distant) past. Has anybody seen this recently? Many years ago I had a diff to fix it. Lost the diff, but I remember it's not super complicated. I figured the problem went away (due to less tight partitioning) and haven't seen it myself. If people are still having to workaround it, I can cook something up.
Re: install(1) could fail due to race
On Mon, 07 Jan 2019 22:55:54 +, Christian Weisgerber wrote: > Oooh, that short period can be the time between disk flushes (30s?) > with softupdates. I think that's one of the scenarios where you > can run out of disk space if you have softupdates enabled. Yes, I think I've had that happen in the (now rather distant) past. - todd
Re: sbin/wsconsctl: show more data
Mon, 07 Jan 2019 16:27:46 -0700 "Theo de Raadt" > > use stty -a > Also, probably complementary methods could be considered: $ tput co li $ wsfontload -l
snmpd: dup open ttys to /dev/null in demons mode
Hi,
This diff is similar to bluhm@'s fix for httpd and relayd, but for snmpd.
> During the fork+exec implementation, daemon(3) was moved after
> proc_init(). As a consequence httpd(8) and relayd(8) child processes
> do not detach from the terminal anymore. Dup /dev/null to the stdio
> file descriptors in the children.
bye,
Jan
Index: proc.c
===
RCS file: /cvs/src/usr.sbin/snmpd/proc.c,v
retrieving revision 1.25
diff -u -p -r1.25 proc.c
--- proc.c 5 Aug 2018 09:33:13 - 1.25
+++ proc.c 8 Jan 2019 00:29:29 -
@@ -29,13 +29,14 @@
#include
#include
#include
+#include
#include
#include
#include
#include "snmpd.h"
-voidproc_exec(struct privsep *, struct privsep_proc *, unsigned int,
+voidproc_exec(struct privsep *, struct privsep_proc *, unsigned int, int,
int, char **);
voidproc_setup(struct privsep *, struct privsep_proc *, unsigned int);
voidproc_open(struct privsep *, int, int);
@@ -80,7 +81,7 @@ proc_getid(struct privsep_proc *procs, u
void
proc_exec(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
-int argc, char **argv)
+int debug, int argc, char **argv)
{
unsigned int proc, nargc, i, proc_i;
char**nargv;
@@ -141,6 +142,16 @@ proc_exec(struct privsep *ps, struct pri
} else if (fcntl(fd, F_SETFD, 0) == -1)
fatal("fcntl");
+ /* Daemons detach from terminal. */
+ if (!debug && (fd =
+ open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
+ (void)dup2(fd, STDIN_FILENO);
+ (void)dup2(fd, STDOUT_FILENO);
+ (void)dup2(fd, STDERR_FILENO);
+ if (fd > 2)
+ (void)close(fd);
+ }
+
execvp(argv[0], nargv);
fatal("%s: execvp", __func__);
break;
@@ -191,7 +202,7 @@ proc_connect(struct privsep *ps)
void
proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
-int argc, char **argv, enum privsep_procid proc_id)
+int debug, int argc, char **argv, enum privsep_procid proc_id)
{
struct privsep_proc *p = NULL;
struct privsep_pipes*pa, *pb;
@@ -231,7 +242,7 @@ proc_init(struct privsep *ps, struct pri
}
/* Engage! */
- proc_exec(ps, procs, nproc, argc, argv);
+ proc_exec(ps, procs, nproc, debug, argc, argv);
return;
}
Index: snmpd.c
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpd.c,v
retrieving revision 1.40
diff -u -p -r1.40 snmpd.c
--- snmpd.c 5 Nov 2018 11:59:05 - 1.40
+++ snmpd.c 8 Jan 2019 00:29:02 -
@@ -230,7 +230,7 @@ main(int argc, char *argv[])
pf_init();
snmpd_generate_engineid(env);
- proc_init(ps, procs, nitems(procs), argc0, argv0, proc_id);
+ proc_init(ps, procs, nitems(procs), debug, argc0, argv0, proc_id);
if (!debug && daemon(0, 0) == -1)
err(1, "failed to daemonize");
Index: snmpd.h
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v
retrieving revision 1.80
diff -u -p -r1.80 snmpd.h
--- snmpd.h 5 Aug 2018 09:33:13 - 1.80
+++ snmpd.h 8 Jan 2019 00:25:01 -
@@ -762,7 +762,7 @@ void usm_make_report(struct snmp_messa
/* proc.c */
enum privsep_procid
proc_getid(struct privsep_proc *, unsigned int, const char *);
-voidproc_init(struct privsep *, struct privsep_proc *, unsigned int,
+voidproc_init(struct privsep *, struct privsep_proc *, unsigned int, int,
int, char **, enum privsep_procid);
voidproc_kill(struct privsep *);
voidproc_connect(struct privsep *);
Re: sbin/wsconsctl: show more data
Ted Unangst wrote: > Artturi Alm wrote: > > display.width=1920 > > display.height=1200 > > display.depth=32 > > display.emulations=vt100 > > display.col_x_row=120x37 > > display.font_wxh=16x32 > > now that we've all had a chance to weigh in on the font, why are you adding > this weird x format? speaking of ugly... it should print columns and rows just > like width and height. it is a waste of kernel and userland code since another mechanism already exists use stty -a
Re: install(1) could fail due to race
On 2019-01-07, "Todd C. Miller" wrote: > The main difference with -S is that you end up with two copies of > the target file on disk for a short period of time. Oooh, that short period can be the time between disk flushes (30s?) with softupdates. I think that's one of the scenarios where you can run out of disk space if you have softupdates enabled. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: install(1) could fail due to race
On Mon, Jan 07 2019 16:32:31 -0500, Ted Unangst wrote: > > > This doubles the number of synchronous > > > file operations. > > > > Does it? Without safecopy, the operations performed are: > > > > unlink(targetfile); > > open(targetfile, O_CREAT|O_EXCL); > > write(); > > fchmod(); > > close(); > > > > with safecopy, they are: > > > > open(tempfile, O_CREAT|O_EXCL); > > write(); > > fchmod(); > > close(); > > rename(tempfile, targetfile); > > > > which to me seems identical in the number of file syscalls made. > > oh, I think I forgot to count the unlink(). rename() within a directory is > about the same cost as unlink(), so the two cases do seem equal. great, we're on the same page :) here's an additional manpage adjustment on top, that I somehow missed the first time around. diff --git a/usr.bin/xinstall/install.1 b/usr.bin/xinstall/install.1 index f7d8707ce6f..64f99504752 100644 --- a/usr.bin/xinstall/install.1 +++ b/usr.bin/xinstall/install.1 @@ -101,7 +101,7 @@ Create directories. Missing parent directories are created as required. This option cannot be used with the .Fl B , b , C , c , -.Fl f , p , S , +.Fl f , p , or .Fl s options. @@ -198,9 +198,8 @@ The .Fl C , .Fl D , .Fl F , -.Fl p , and -.Fl S +.Fl p flags are non-standard and should not be relied upon for portability. .Pp Temporary files may be left in the target directory if -- Lauri Tirkkonen | lotheac @ IRCnet
diff: new PCI Vendor and Product ID for RocketPort serial card
Hi,
the following diff adds the PCI vendor and product ID for a PCI
multiport serial card.
dmesg shows the device as:
"Comtrol Corporation RocketPort PCI 16-port Serial" rev 0x01 at pci1 dev
1 function 0 not configured
bye,
Jan
Index: pci/pcidevs
===
RCS file: /cvs/src/sys/dev/pci/pcidevs,v
retrieving revision 1.1874
diff -u -p -r1.1874 pcidevs
--- pci/pcidevs 5 Jan 2019 11:49:41 - 1.1874
+++ pci/pcidevs 7 Jan 2019 20:23:36 -
@@ -194,6 +194,7 @@ vendor AD 0x11d4 Analog Devices
vendor ZORAN 0x11de Zoran
vendor PIJNENBURG 0x11e3 Pijnenburg
vendor COMPEX 0x11f6 Compex
+vendor COMCORP 0x11fe Comtrol Corporation
vendor CYCLADES0x120e Cyclades
vendor ESSENTIAL 0x120f Essential Communications
vendor O2MICRO 0x1217 O2 Micro
@@ -2423,6 +2424,9 @@ product COMPAQ NF3P_BNC 0xf150 NetFlex
product COMPEX COMPEXE 0x1401 Compexe
product COMPEX RL100ATX0x2011 RL100-ATX 10/100
product COMPEX 98713 0x9881 PMAC 98713
+
+/* Comtrol Corporation */
+product COMCORP ROCKETPORT_16 0x0009 RocketPort PCI 16-port Serial
/* Conexant products */
product CONEXANT 56K_WINMODEM 0x1033 56k Winmodem
Index: pci/pcidevs.h
===
RCS file: /cvs/src/sys/dev/pci/pcidevs.h,v
retrieving revision 1.1867
diff -u -p -r1.1867 pcidevs.h
--- pci/pcidevs.h 5 Jan 2019 11:50:32 - 1.1867
+++ pci/pcidevs.h 7 Jan 2019 20:23:37 -
@@ -199,6 +199,7 @@
#definePCI_VENDOR_ZORAN0x11de /* Zoran */
#definePCI_VENDOR_PIJNENBURG 0x11e3 /* Pijnenburg */
#definePCI_VENDOR_COMPEX 0x11f6 /* Compex */
+#definePCI_VENDOR_COMCORP 0x11fe /* Comtrol Corporation
*/
#definePCI_VENDOR_CYCLADES 0x120e /* Cyclades */
#definePCI_VENDOR_ESSENTIAL0x120f /* Essential
Communications */
#definePCI_VENDOR_O2MICRO 0x1217 /* O2 Micro */
@@ -2428,6 +2429,9 @@
#definePCI_PRODUCT_COMPEX_COMPEXE 0x1401 /* Compexe */
#definePCI_PRODUCT_COMPEX_RL100ATX 0x2011 /* RL100-ATX
10/100 */
#definePCI_PRODUCT_COMPEX_987130x9881 /* PMAC 98713 */
+
+/* Comtrol Corporation */
+#definePCI_PRODUCT_COMCORP_ROCKETPORT_16 0x0009 /*
RocketPort PCI 16-port Serial */
/* Conexant products */
#definePCI_PRODUCT_CONEXANT_56K_WINMODEM 0x1033 /* 56k
Winmodem */
Index: pci/pcidevs_data.h
===
RCS file: /cvs/src/sys/dev/pci/pcidevs_data.h,v
retrieving revision 1.1862
diff -u -p -r1.1862 pcidevs_data.h
--- pci/pcidevs_data.h 5 Jan 2019 11:50:32 - 1.1862
+++ pci/pcidevs_data.h 7 Jan 2019 20:23:37 -
@@ -7680,6 +7680,10 @@ static const struct pci_known_product pc
"PMAC 98713",
},
{
+ PCI_VENDOR_COMCORP, PCI_PRODUCT_COMCORP_ROCKETPORT_16,
+ "RocketPort PCI 16-port Serial",
+ },
+ {
PCI_VENDOR_CONEXANT, PCI_PRODUCT_CONEXANT_56K_WINMODEM,
"56k Winmodem",
},
@@ -28734,6 +28738,10 @@ static const struct pci_known_vendor pci
{
PCI_VENDOR_COMPEX,
"Compex",
+ },
+ {
+ PCI_VENDOR_COMCORP,
+ "Comtrol Corporation",
},
{
PCI_VENDOR_CYCLADES,
Re: install(1) could fail due to race
Lauri Tirkkonen wrote: > On Mon, Jan 07 2019 15:48:25 -0500, Ted Unangst wrote: > > Lauri Tirkkonen wrote: > > > Hi, it seems install(1) has a race condition: in create_newfile, it > > > first unlinks the target file and then tries to open it with > > > O_CREAT|O_EXCL. > > > > > The below diff essentially removes the -S option and makes install > > > always use temp files (ie. -S is always on), eliminating the race since > > > rename(2) cannot fail like this. > > > > I don't know. Presumably if there weren't any downside to safecopy, it would > > already have been made the default. > > I had this thought, but could not think of any real downsides... > > > This doubles the number of synchronous > > file operations. > > Does it? Without safecopy, the operations performed are: > > unlink(targetfile); > open(targetfile, O_CREAT|O_EXCL); > write(); > fchmod(); > close(); > > with safecopy, they are: > > open(tempfile, O_CREAT|O_EXCL); > write(); > fchmod(); > close(); > rename(tempfile, targetfile); > > which to me seems identical in the number of file syscalls made. oh, I think I forgot to count the unlink(). rename() within a directory is about the same cost as unlink(), so the two cases do seem equal.
Re: sbin/wsconsctl: show more data
Artturi Alm wrote: > display.width=1920 > display.height=1200 > display.depth=32 > display.emulations=vt100 > display.col_x_row=120x37 > display.font_wxh=16x32 now that we've all had a chance to weigh in on the font, why are you adding this weird x format? speaking of ugly... it should print columns and rows just like width and height.
Re: install(1) could fail due to race
The main difference with -S is that you end up with two copies of the target file on disk for a short period of time. That was a bigger deal back when -S was added. Today, it probably doesn't matter. - todd
Re: install(1) could fail due to race
On Mon, Jan 07 2019 15:48:25 -0500, Ted Unangst wrote: > Lauri Tirkkonen wrote: > > Hi, it seems install(1) has a race condition: in create_newfile, it > > first unlinks the target file and then tries to open it with > > O_CREAT|O_EXCL. > > > The below diff essentially removes the -S option and makes install > > always use temp files (ie. -S is always on), eliminating the race since > > rename(2) cannot fail like this. > > I don't know. Presumably if there weren't any downside to safecopy, it would > already have been made the default. I had this thought, but could not think of any real downsides... > This doubles the number of synchronous > file operations. Does it? Without safecopy, the operations performed are: unlink(targetfile); open(targetfile, O_CREAT|O_EXCL); write(); fchmod(); close(); with safecopy, they are: open(tempfile, O_CREAT|O_EXCL); write(); fchmod(); close(); rename(tempfile, targetfile); which to me seems identical in the number of file syscalls made. -- Lauri Tirkkonen | lotheac @ IRCnet
Re: grep: convert fgetln to getline
On Mon, Jan 07 2019 15:41:53 -0500, Ted Unangst wrote:
> Lauri Tirkkonen wrote:
> > On Sun, Jan 06 2019 14:02:16 -0500, Ted Unangst wrote:
> > > Lauri Tirkkonen wrote:
> > > > Hi, another simple diff converting fgetln usage to getline.
> > > >
> > > > I also considered converting grep_fgetln to grep_getline, but that would
> > > > mean that for FILE_MMAP we'd have to copy the string. So this diff keeps
> > > > the grep_fgetln interface as is, but avoids using fgetln from libc (and
> > > > adds error handling for FILE_STDIO).
> > >
> > > this looks good and seems to work. thanks.
> >
> > actually, it doesn't work :) the added error handling catches that
> > directory fd's are being passed into grep_fgetln, causing an error exit
> > with grep -r (or just grep foo /etc/*). I'm working on a second diff to
> > refactor the file handling a bit.
>
> oh, interesting. that's sloppy. can you please fix that first, separately?
Sure, here it is. The idea is to use grep_fdopen for everything, making
grep_open just open the file and pass the fd to grep_fdopen, do fstat()
in grep_fdopen and modify mmopen() accordingly (it no longer needs to
open the file or stat it).
grep_tree is modified to not call procfile() on FTS_D (FTS_DP was
already there). In addition grep_fdopen returns NULL and sets errno ==
EISDIR if it encounters a directory; this can happen if a directory
argument is given on command line, or for recursive grep, if the file
being processed is a symlink to a directory, which fts doesn't notice.
procfile() handles the EISDIR return silently, matching current
behavior.
I also removed the useless mode argument from grep_open and grep_fdopen;
we always open files read-only.
diff --git a/usr.bin/grep/file.c b/usr.bin/grep/file.c
index 4b3c689e4ab..2befc4304c1 100644
--- a/usr.bin/grep/file.c
+++ b/usr.bin/grep/file.c
@@ -26,7 +26,10 @@
* SUCH DAMAGE.
*/
+#include
#include
+#include
+#include
#include
#include
#include
@@ -90,68 +93,64 @@ gzfgetln(gzFile *f, size_t *len)
#endif
file_t *
-grep_fdopen(int fd, char *mode)
+grep_fdopen(int fd)
{
file_t *f;
+ struct stat sb;
if (fd == STDIN_FILENO)
snprintf(fname, sizeof fname, "(standard input)");
- else
+ else if (fname[0] == '\0')
snprintf(fname, sizeof fname, "(fd %d)", fd);
+ if (fstat(fd, &sb) == -1)
+ return NULL;
+ if (S_ISDIR(sb.st_mode)) {
+ errno = EISDIR;
+ return NULL;
+ }
+
f = grep_malloc(sizeof *f);
#ifndef NOZ
if (Zflag) {
f->type = FILE_GZIP;
f->noseek = lseek(fd, 0L, SEEK_SET) == -1;
- if ((f->gzf = gzdopen(fd, mode)) != NULL)
+ if ((f->gzf = gzdopen(fd, "r")) != NULL)
return f;
- } else
+ }
#endif
- {
- f->type = FILE_STDIO;
- f->noseek = isatty(fd);
- if ((f->f = fdopen(fd, mode)) != NULL)
- return f;
+ f->noseek = isatty(fd);
+#ifndef SMALL
+ /* try mmap first; if it fails, try stdio */
+ if (!f->noseek && (f->mmf = mmopen(fd, &sb)) != NULL) {
+ f->type = FILE_MMAP;
+ return f;
}
+#endif
+ f->type = FILE_STDIO;
+ if ((f->f = fdopen(fd, "r")) != NULL)
+ return f;
free(f);
return NULL;
}
file_t *
-grep_open(char *path, char *mode)
+grep_open(char *path)
{
file_t *f;
+ int fd;
snprintf(fname, sizeof fname, "%s", path);
- f = grep_malloc(sizeof *f);
- f->noseek = 0;
-
-#ifndef NOZ
- if (Zflag) {
- f->type = FILE_GZIP;
- if ((f->gzf = gzopen(fname, mode)) != NULL)
- return f;
- } else
-#endif
- {
-#ifndef SMALL
- /* try mmap first; if it fails, try stdio */
- if ((f->mmf = mmopen(fname, mode)) != NULL) {
- f->type = FILE_MMAP;
- return f;
- }
-#endif
- f->type = FILE_STDIO;
- if ((f->f = fopen(path, mode)) != NULL)
- return f;
- }
+ if ((fd = open(fname, O_RDONLY)) == -1)
+ return NULL;
- free(f);
- return NULL;
+ f = grep_fdopen(fd);
+ if (f == NULL)
+ close(fd);
+ return f;
}
int
diff --git a/usr.bin/grep/grep.c b/usr.bin/grep/grep.c
index 913cc97a0f3..e5e48fa163a 100644
--- a/usr.bin/grep/grep.c
+++ b/usr.bin/grep/grep.c
@@ -63,9 +63,7 @@ intFflag; /* -F: interpret pattern as list of
fixed strings */
int Hflag; /* -H: always print filename header */
int Lflag; /* -L: only show names of files with no matches */
int Rflag; /* -R: recursively search directory trees */
-#ifndef NOZ
int Zflag; /* -Z: decompress input before processing */
-
Re: install(1) could fail due to race
Lauri Tirkkonen wrote: > Hi, it seems install(1) has a race condition: in create_newfile, it > first unlinks the target file and then tries to open it with > O_CREAT|O_EXCL. > The below diff essentially removes the -S option and makes install > always use temp files (ie. -S is always on), eliminating the race since > rename(2) cannot fail like this. I don't know. Presumably if there weren't any downside to safecopy, it would already have been made the default. This doubles the number of synchronous file operations. You should probably fix your project to use -S instead.
Re: grep: convert fgetln to getline
Lauri Tirkkonen wrote: > On Sun, Jan 06 2019 14:02:16 -0500, Ted Unangst wrote: > > Lauri Tirkkonen wrote: > > > Hi, another simple diff converting fgetln usage to getline. > > > > > > I also considered converting grep_fgetln to grep_getline, but that would > > > mean that for FILE_MMAP we'd have to copy the string. So this diff keeps > > > the grep_fgetln interface as is, but avoids using fgetln from libc (and > > > adds error handling for FILE_STDIO). > > > > this looks good and seems to work. thanks. > > actually, it doesn't work :) the added error handling catches that > directory fd's are being passed into grep_fgetln, causing an error exit > with grep -r (or just grep foo /etc/*). I'm working on a second diff to > refactor the file handling a bit. oh, interesting. that's sloppy. can you please fix that first, separately? (wrt getline, theo raised some concern that getline copies more data than fgetln does. we care quite a bit about grep performance, so we need to consider that some more.)
Re: kcov: enable retguard
That's great!
> Hi,
> The problems caused by enabling both kcov and retguard was due to the
> increased kernel size. Now that NKL2_KIMG_ENTRIES has been bumped on
> amd64, it's no longer a problem.
>
> Comments? OK?
>
> Index: arch/amd64/conf/Makefile.amd64
> ===
> RCS file: /cvs/src/sys/arch/amd64/conf/Makefile.amd64,v
> retrieving revision 1.107
> diff -u -p -r1.107 Makefile.amd64
> --- arch/amd64/conf/Makefile.amd6430 Dec 2018 23:08:05 - 1.107
> +++ arch/amd64/conf/Makefile.amd647 Jan 2019 19:25:07 -
> @@ -95,7 +95,6 @@ LINKFLAGS+= -S
> .endif
>
> .if ${SYSTEM_OBJ:Mkcov.o} && ${COMPILER_VERSION:Mclang}
> -CMACHFLAGS+= -fno-ret-protector
> PROF=-fsanitize-coverage=trace-pc
> .endif
>
kcov: enable retguard
Hi,
The problems caused by enabling both kcov and retguard was due to the
increased kernel size. Now that NKL2_KIMG_ENTRIES has been bumped on
amd64, it's no longer a problem.
Comments? OK?
Index: arch/amd64/conf/Makefile.amd64
===
RCS file: /cvs/src/sys/arch/amd64/conf/Makefile.amd64,v
retrieving revision 1.107
diff -u -p -r1.107 Makefile.amd64
--- arch/amd64/conf/Makefile.amd64 30 Dec 2018 23:08:05 - 1.107
+++ arch/amd64/conf/Makefile.amd64 7 Jan 2019 19:25:07 -
@@ -95,7 +95,6 @@ LINKFLAGS+= -S
.endif
.if ${SYSTEM_OBJ:Mkcov.o} && ${COMPILER_VERSION:Mclang}
-CMACHFLAGS+= -fno-ret-protector
PROF= -fsanitize-coverage=trace-pc
.endif
Re: grep: convert fgetln to getline
On Sun, Jan 06 2019 14:02:16 -0500, Ted Unangst wrote: > Lauri Tirkkonen wrote: > > Hi, another simple diff converting fgetln usage to getline. > > > > I also considered converting grep_fgetln to grep_getline, but that would > > mean that for FILE_MMAP we'd have to copy the string. So this diff keeps > > the grep_fgetln interface as is, but avoids using fgetln from libc (and > > adds error handling for FILE_STDIO). > > this looks good and seems to work. thanks. actually, it doesn't work :) the added error handling catches that directory fd's are being passed into grep_fgetln, causing an error exit with grep -r (or just grep foo /etc/*). I'm working on a second diff to refactor the file handling a bit. -- Lauri Tirkkonen | lotheac @ IRCnet
Re: install(1) could fail due to race
On Mon, Jan 07 2019 20:13:09 +0200, Lauri Tirkkonen wrote:
> @@ -128,9 +127,6 @@ main(int argc, char *argv[])
> case 'p':
> docompare = dopreserve = 1;
> break;
> - case 'S':
> - safecopy = 1;
> - break;
> case 's':
> dostrip = 1;
> break;
I realized -S is used in many places throughout src (including in
xinstall/Makefile itself, and in bsd.lib.mk, bsd.prog.mk), so -S should
probably be accepted for compatibility until those -S's can be removed.
diff revised as such.
diff --git a/usr.bin/xinstall/install.1 b/usr.bin/xinstall/install.1
index fd5db0abc37..f7d8707ce6f 100644
--- a/usr.bin/xinstall/install.1
+++ b/usr.bin/xinstall/install.1
@@ -38,7 +38,7 @@
.Nd install binaries
.Sh SYNOPSIS
.Nm install
-.Op Fl bCcDdFpSs
+.Op Fl bCcDdFps
.Op Fl B Ar suffix
.Op Fl f Ar flags
.Op Fl g Ar group
@@ -139,17 +139,6 @@ Copy the file, as if the
(compare and copy) option is specified,
except if the target file doesn't already exist or is different,
then preserve the modification time of the file.
-.It Fl S
-Safe copy.
-Normally,
-.Nm
-unlinks an existing target before installing the new file.
-With the
-.Fl S
-flag a temporary file is used and then renamed to be
-the target.
-The reason this is safer is that if the copy or
-rename fails, the existing target is left untouched.
.It Fl s
.Nm
exec's the command
@@ -186,18 +175,8 @@ Default is
.Sh FILES
.Bl -tag -width INS@XX -compact
.It Pa INS@XX
-If either
-.Fl S
-option is specified, or the
-.Fl C
-or
-.Fl p
-option is used in conjunction with the
-.Fl s
-option, temporary files named INS@XX,
-where XX is decided by
-.Xr mkstemp 3 ,
-are created in the target directory.
+Temporary files created in the target directory by
+.Xr mkstemp 3 .
.El
.Sh EXIT STATUS
.Ex -std install
diff --git a/usr.bin/xinstall/xinstall.c b/usr.bin/xinstall/xinstall.c
index c08d82eeed4..be7311b354a 100644
--- a/usr.bin/xinstall/xinstall.c
+++ b/usr.bin/xinstall/xinstall.c
@@ -60,7 +60,7 @@
#define NOCHANGEBITS (UF_IMMUTABLE | UF_APPEND | SF_IMMUTABLE | SF_APPEND)
#define BACKUP_SUFFIX ".old"
-int dobackup, docompare, dodest, dodir, dopreserve, dostrip, safecopy;
+int dobackup, docompare, dodest, dodir, dopreserve, dostrip;
int mode = S_IRWXU|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH;
char pathbuf[PATH_MAX], tempfile[PATH_MAX];
char *suffix = BACKUP_SUFFIX;
@@ -73,7 +73,6 @@ void install(char *, char *, u_long, u_int);
void install_dir(char *, int);
void strip(char *);
void usage(void);
-intcreate_newfile(char *, struct stat *);
intcreate_tempfile(char *, char *, size_t);
intfile_write(int, char *, size_t, int *, int *, int);
void file_flush(int, int);
@@ -129,7 +128,6 @@ main(int argc, char *argv[])
docompare = dopreserve = 1;
break;
case 'S':
- safecopy = 1;
break;
case 's':
dostrip = 1;
@@ -148,17 +146,13 @@ main(int argc, char *argv[])
argv += optind;
/* some options make no sense when creating directories */
- if ((safecopy || docompare || dostrip) && dodir)
+ if ((docompare || dostrip) && dodir)
usage();
/* must have at least two arguments, except when creating directories */
if (argc < 2 && !dodir)
usage();
- /* need to make a temp copy so we can compare stripped version */
- if (docompare && dostrip)
- safecopy = 1;
-
/* get group and owner id's */
if (group != NULL && gid_from_group(group, &gid) == -1) {
gid = strtonum(group, 0, GID_MAX, &errstr);
@@ -265,54 +259,30 @@ install(char *from_name, char *to_name, u_long fset,
u_int flags)
err(1, "%s", from_name);
}
- if (safecopy) {
- to_fd = create_tempfile(to_name, tempfile, sizeof(tempfile));
- if (to_fd < 0)
- err(1, "%s", tempfile);
- } else if (docompare && !dostrip) {
- if ((to_fd = open(to_name, O_RDONLY, 0)) < 0)
- err(1, "%s", to_name);
- } else {
- if ((to_fd = create_newfile(to_name, &to_sb)) < 0)
- err(1, "%s", to_name);
- }
+ to_fd = create_tempfile(to_name, tempfile, sizeof(tempfile));
+ if (to_fd < 0)
+ err(1, "%s", tempfile);
- if (!devnull) {
- if (docompare && !safecopy) {
- files_match = !(compare(from_fd, from_name,
- from_sb.st_size, to_fd,
- to_name, to_sb.st_size));
-
- /* Truncate "to" file for copy unless we match */
- if (!f
install(1) could fail due to race
Hi, it seems install(1) has a race condition: in create_newfile, it
first unlinks the target file and then tries to open it with
O_CREAT|O_EXCL.
Normally this would not be a problem, but I actually ran into this when
parallel building gcc 6.4.0 (on another OS, but we use OpenBSD
install(1)): they seem to install the same set of headers at least
twice. Granted, that doesn't make any sense, but that's what their build
seems to do...
Easy way to repro this failure is to use a Makefile like:
all: 1 2 3 4 5 6 7 8
1:
install /etc/passwd .
2:
install /etc/passwd .
3:
install /etc/passwd .
4:
install /etc/passwd .
5:
install /etc/passwd .
6:
install /etc/passwd .
7:
install /etc/passwd .
8:
install /etc/passwd .
and run while make -j8; do :; done
The below diff essentially removes the -S option and makes install
always use temp files (ie. -S is always on), eliminating the race since
rename(2) cannot fail like this.
diff --git a/usr.bin/xinstall/install.1 b/usr.bin/xinstall/install.1
index fd5db0abc37..f7d8707ce6f 100644
--- a/usr.bin/xinstall/install.1
+++ b/usr.bin/xinstall/install.1
@@ -38,7 +38,7 @@
.Nd install binaries
.Sh SYNOPSIS
.Nm install
-.Op Fl bCcDdFpSs
+.Op Fl bCcDdFps
.Op Fl B Ar suffix
.Op Fl f Ar flags
.Op Fl g Ar group
@@ -139,17 +139,6 @@ Copy the file, as if the
(compare and copy) option is specified,
except if the target file doesn't already exist or is different,
then preserve the modification time of the file.
-.It Fl S
-Safe copy.
-Normally,
-.Nm
-unlinks an existing target before installing the new file.
-With the
-.Fl S
-flag a temporary file is used and then renamed to be
-the target.
-The reason this is safer is that if the copy or
-rename fails, the existing target is left untouched.
.It Fl s
.Nm
exec's the command
@@ -186,18 +175,8 @@ Default is
.Sh FILES
.Bl -tag -width INS@XX -compact
.It Pa INS@XX
-If either
-.Fl S
-option is specified, or the
-.Fl C
-or
-.Fl p
-option is used in conjunction with the
-.Fl s
-option, temporary files named INS@XX,
-where XX is decided by
-.Xr mkstemp 3 ,
-are created in the target directory.
+Temporary files created in the target directory by
+.Xr mkstemp 3 .
.El
.Sh EXIT STATUS
.Ex -std install
diff --git a/usr.bin/xinstall/xinstall.c b/usr.bin/xinstall/xinstall.c
index c08d82eeed4..b6f5c595646 100644
--- a/usr.bin/xinstall/xinstall.c
+++ b/usr.bin/xinstall/xinstall.c
@@ -60,7 +60,7 @@
#define NOCHANGEBITS (UF_IMMUTABLE | UF_APPEND | SF_IMMUTABLE | SF_APPEND)
#define BACKUP_SUFFIX ".old"
-int dobackup, docompare, dodest, dodir, dopreserve, dostrip, safecopy;
+int dobackup, docompare, dodest, dodir, dopreserve, dostrip;
int mode = S_IRWXU|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH;
char pathbuf[PATH_MAX], tempfile[PATH_MAX];
char *suffix = BACKUP_SUFFIX;
@@ -73,7 +73,6 @@ void install(char *, char *, u_long, u_int);
void install_dir(char *, int);
void strip(char *);
void usage(void);
-intcreate_newfile(char *, struct stat *);
intcreate_tempfile(char *, char *, size_t);
intfile_write(int, char *, size_t, int *, int *, int);
void file_flush(int, int);
@@ -128,9 +127,6 @@ main(int argc, char *argv[])
case 'p':
docompare = dopreserve = 1;
break;
- case 'S':
- safecopy = 1;
- break;
case 's':
dostrip = 1;
break;
@@ -148,17 +144,13 @@ main(int argc, char *argv[])
argv += optind;
/* some options make no sense when creating directories */
- if ((safecopy || docompare || dostrip) && dodir)
+ if ((docompare || dostrip) && dodir)
usage();
/* must have at least two arguments, except when creating directories */
if (argc < 2 && !dodir)
usage();
- /* need to make a temp copy so we can compare stripped version */
- if (docompare && dostrip)
- safecopy = 1;
-
/* get group and owner id's */
if (group != NULL && gid_from_group(group, &gid) == -1) {
gid = strtonum(group, 0, GID_MAX, &errstr);
@@ -265,54 +257,30 @@ install(char *from_name, char *to_name, u_long fset,
u_int flags)
err(1, "%s", from_name);
}
- if (safecopy) {
- to_fd = create_tempfile(to_name, tempfile, sizeof(tempfile));
- if (to_fd < 0)
- err(1, "%s", tempfile);
- } else if (docompare && !dostrip) {
- if ((to_fd = open(to_name, O_RDONLY, 0)) < 0)
- err(1, "%s", to_name);
- } else {
- if ((to_fd = create_newfile(to_name, &to_sb)) < 0)
- err(1, "%s", to_name);
- }
+ to_fd = create_tempfile(to_
Re: pci/i386: Fix typo
On Thursday, 13. December 2018, 19:52:13 Mark Kettenis wrote:
> > From: Christian Ludwig
> > Cc: Christian Ludwig
> > Date: Thu, 13 Dec 2018 17:44:47 +0100
> >
> > Extents code has its own set of flags and does not use malloc's.
> > The current code passes in EX_FAST by accident, which does no harm.
> > That's probably why nobody stumbled upon it, yet.
> > ---
> >
> > sys/arch/i386/pci/pci_machdep.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
>
> ok kettenis@
Anyone in the mood to commit this?
> > diff --git a/sys/arch/i386/pci/pci_machdep.c
> > b/sys/arch/i386/pci/pci_machdep.c index b712ff242ed..bfd8055f125 100644
> > --- a/sys/arch/i386/pci/pci_machdep.c
> > +++ b/sys/arch/i386/pci/pci_machdep.c
> > @@ -915,7 +915,7 @@ pci_init_extents(void)
> >
> > NULL, 0, EX_NOWAIT | EX_FILLED);
> >
> > if (pciio_ex == NULL)
> >
> > return;
> >
> > - extent_free(pciio_ex, 0, 0x1, M_NOWAIT);
> > + extent_free(pciio_ex, 0, 0x1, EX_NOWAIT);
> >
> > }
> >
> > if (pcimem_ex == NULL) {
Re: http://cvshome.org no longer related to CVS version control.
Sorry, not man page. I mean running just cvs command: The Concurrent Versions System (CVS) is a tool for version control. For CVS updates and additional information, see the CVS home page at http://www.cvshome.org/ or Pascal Molli's CVS site at http://www.loria.fr/~molli/cvs-index.html On Mon, 7 Jan 2019 at 11:01, Matteo Niccoli wrote: > Hello, > > reading the cvs man page I realized that there are a lot of references to > the cvshome.org site , but it is not the cvs home page anymore. Maybe it > should be changed to https://www.nongnu.org/cvs/? >
http://cvshome.org no longer related to CVS version control.
Hello, reading the cvs man page I realized that there are a lot of references to the cvshome.org site , but it is not the cvs home page anymore. Maybe it should be changed to https://www.nongnu.org/cvs/?
Re: replacing timeout_add() with timeout_add_msec()
> Date: Sun, 6 Jan 2019 21:46:01 -0600 > From: Amit Kulkarni > > > > Even on amd64, I won't be able to test, because of missing hardware. > > > If you think something is wrong, please will you let me have your > > > feedback? > > > > I'm a bit stunned at the zeal to push untested diffs into the tree > > > > (you didn't ask anyone to test it for you) > > I requested for critical review or feedback in the initial email, to > know if I am going down the right path. If the approach is ok, then > I was planning to sit down to try converting all the straightforward > of timeout_add() to timeout_add_msec() calls in the tree, then a > review again, before finally requesting a test run. You can't do these conversions without understanding the code. In particular, timeout_add(t, 1) could mean sleep for 1/HZ seconds *or* sleep as short as possible. It is actually most likely the latter, in which case coverting to timeout_add_msec() makes no sense.
