cdio, ftp: use monotime for progress logging
Hey, Use the monotonic clock when logging progress in cdio(1) and ftp(1). This keeps the progress log from blipping or stalling if the system time is changed, e.g., in the midst of a transfer or rip. ok? -- Scott Cheloha Index: usr.bin/cdio/mmc.c === RCS file: /cvs/src/usr.bin/cdio/mmc.c,v retrieving revision 1.30 diff -u -p -r1.30 mmc.c --- usr.bin/cdio/mmc.c 16 Jan 2015 06:40:06 - 1.30 +++ usr.bin/cdio/mmc.c 23 Dec 2017 02:24:07 - @@ -16,6 +16,7 @@ */ #include +#include #include #include #include /* setbit, isset */ @@ -27,6 +28,7 @@ #include #include #include +#include #include #include "extern.h" @@ -433,7 +435,7 @@ writetao(struct track_head *thp) int writetrack(struct track_info *tr, int track) { - struct timeval tv, otv, atv; + struct timespec ts, ots, ats; u_char databuf[65536], nblk; u_int end_lba, lba, tmp; scsireq_t scr; @@ -451,9 +453,9 @@ writetrack(struct track_info *tr, int tr scr.senselen = SENSEBUFLEN; scr.flags = SCCMD_ESCAPE|SCCMD_WRITE; - timerclear(); - atv.tv_sec = 1; - atv.tv_usec = 0; + timespecclear(); + ats.tv_sec = 1; + ats.tv_nsec = 0; if (get_nwa() != SCCMD_OK) { warnx("cannot get next writable address"); @@ -500,13 +502,13 @@ again: } lba += nblk; - gettimeofday(, NULL); - if (lba == end_lba || timercmp(, , >)) { + clock_gettime(CLOCK_MONOTONIC, ); + if (lba == end_lba || timespeccmp(, , >)) { fprintf(stderr, "\rtrack %02d '%c' %08u/%08u %3d%%", track, tr->type, lba, end_lba, 100 * lba / end_lba); - timeradd(, , ); + timespecadd(, , ); } tmp = htobe32(lba); /* update lba in cdb */ memcpy([2], , sizeof(tmp)); Index: usr.bin/cdio/rip.c === RCS file: /cvs/src/usr.bin/cdio/rip.c,v retrieving revision 1.16 diff -u -p -r1.16 rip.c --- usr.bin/cdio/rip.c 20 Aug 2015 22:32:41 - 1.16 +++ usr.bin/cdio/rip.c 23 Dec 2017 02:24:07 - @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -37,6 +38,7 @@ #include #include #include +#include #include #include "extern.h" @@ -362,7 +364,7 @@ read_data_sector(u_int32_t lba, u_char * int read_track(struct track *ti) { - struct timeval tv, otv, atv; + struct timespec ts, ots, ats; u_int32_t i, blksize, n_sec; u_char *sec; int error; @@ -373,18 +375,18 @@ read_track(struct track *ti) if (sec == NULL) return (-1); - timerclear(); - atv.tv_sec = 1; - atv.tv_usec = 0; + timespecclear(); + ats.tv_sec = 1; + ats.tv_nsec = 0; for (i = 0; i < n_sec; ) { - gettimeofday(, NULL); - if (timercmp(, , >)) { + clock_gettime(CLOCK_MONOTONIC, ); + if (timespeccmp(, , >)) { fprintf(stderr, "\rtrack %u '%c' %08u/%08u %3u%%", ti->track, (ti->isaudio) ? 'a' : 'd', i, n_sec, 100 * i / n_sec); - timeradd(, , ); + timespecadd(, , ); } error = read_data_sector(i + ti->start_lba, sec, blksize); Index: usr.bin/ftp/util.c === RCS file: /cvs/src/usr.bin/ftp/util.c,v retrieving revision 1.85 diff -u -p -r1.85 util.c --- usr.bin/ftp/util.c 5 Sep 2017 05:37:35 - 1.85 +++ usr.bin/ftp/util.c 23 Dec 2017 02:24:07 - @@ -744,7 +744,7 @@ updateprogressmeter(int signo) * with flag = 0 * - After the transfer, call with flag = 1 */ -static struct timeval start; +static struct timespec start; char *action; @@ -757,21 +757,21 @@ progressmeter(int flag, const char *file */ static const char prefixes[] = " KMGTP"; - static struct timeval lastupdate; + static struct timespec lastupdate; static off_t lastsize; static char *title = NULL; - struct timeval now, td, wait; + struct timespec now, td, wait; off_t cursize, abbrevsize; double elapsed; int ratio, barlength, i, remaining, overhead = 30; char buf[512]; if (flag == -1) { - (void)gettimeofday(, NULL); + clock_gettime(CLOCK_MONOTONIC, ); lastupdate = start; lastsize = restart_point; } -
arp, date, pom, pr: gettimeofday(2) -> time(3)
Hey, None of these programs make use of anything but the .tv_sec field in the timeval initialized by gettimeofday(2), so we can simplify the code some and just use time(3). While here, in date(1) we ought to use err(3) instead of errx(3) if adjtime(2) fails. This: date: adjtime: Operation not permitted is considerably more useful than this: date: adjtime Of some note is that pom(6) was one of the last programs in base calling gettimeofday(2) with a non-NULL timezone argument. ok? -- Scott Cheloha Index: bin/date/date.c === RCS file: /cvs/src/bin/date/date.c,v retrieving revision 1.50 diff -u -p -r1.50 date.c --- bin/date/date.c 19 Oct 2016 18:20:25 - 1.50 +++ bin/date/date.c 23 Dec 2017 01:16:16 - @@ -225,15 +225,10 @@ setthetime(char *p) /* set the time */ if (slidetime) { - struct timeval tv_current; - - if (gettimeofday(_current, NULL) == -1) - err(1, "Could not get local time of day"); - - tv.tv_sec = tval - tv_current.tv_sec; + tv.tv_sec = tval - time(NULL); tv.tv_usec = 0; if (adjtime(, NULL) == -1) - errx(1, "adjtime"); + err(1, "adjtime"); } else { #ifndef SMALL logwtmp("|", "date", ""); Index: games/pom/pom.c === RCS file: /cvs/src/games/pom/pom.c,v retrieving revision 1.25 diff -u -p -r1.25 pom.c --- games/pom/pom.c 1 Dec 2016 20:08:59 - 1.25 +++ games/pom/pom.c 23 Dec 2017 01:16:16 - @@ -44,7 +44,6 @@ * */ -#include #include #include #include @@ -73,8 +72,6 @@ __dead void badformat(void); int main(int argc, char *argv[]) { - struct timeval tp; - struct timezone tzp; struct tm *GMT; time_t tmpt; double days, today, tomorrow; @@ -89,11 +86,8 @@ main(int argc, char *argv[]) strftime(buf, sizeof(buf), "%a %Y %b %e %H:%M:%S (%Z)", localtime()); printf("%s: ", buf); - } else { - if (gettimeofday(,)) - err(1, "gettimeofday"); - tmpt = tp.tv_sec; - } + } else + tmpt = time(NULL); GMT = gmtime(); days = (GMT->tm_yday + 1) + ((GMT->tm_hour + (GMT->tm_min / 60.0) + (GMT->tm_sec / 3600.0)) / 24.0); Index: usr.bin/pr/pr.c === RCS file: /cvs/src/usr.bin/pr/pr.c,v retrieving revision 1.40 diff -u -p -r1.40 pr.c --- usr.bin/pr/pr.c 2 Nov 2017 09:52:04 - 1.40 +++ usr.bin/pr/pr.c 23 Dec 2017 01:16:16 - @@ -34,7 +34,6 @@ */ #include -#include #include #include @@ -45,6 +44,7 @@ #include #include #include +#include #include #include "pr.h" @@ -1442,7 +1442,6 @@ FILE * nxtfile(int argc, char *argv[], char **fname, char *buf, int dt) { FILE *inf = NULL; -struct timeval tv; struct tm *timeptr = NULL; struct stat statbuf; time_t curtime; @@ -1463,14 +1462,7 @@ nxtfile(int argc, char *argv[], char **f *fname = FNAME; if (nohead) return(inf); - if (gettimeofday(, NULL) < 0) { - ++errcnt; - ferrout("pr: cannot get time of day, %s\n", - strerror(errno)); - eoptind = argc - 1; - return(NULL); - } - curtime = tv.tv_sec; + curtime = time(NULL);; timeptr = localtime(); } for (; eoptind < argc; ++eoptind) { @@ -1487,13 +1479,7 @@ nxtfile(int argc, char *argv[], char **f ++eoptind; if (nohead || (dt && twice)) return(inf); - if (gettimeofday(, NULL) < 0) { - ++errcnt; - ferrout("pr: cannot get time of day, %s\n", - strerror(errno)); - return(NULL); - } - curtime = tv.tv_sec; + curtime = time(NULL); timeptr = localtime(); } else { /* @@ -1518,13 +1504,7 @@ nxtfile(int argc, char *argv[], char **f return(inf); if (dt) { - if (gettimeofday(, NULL) < 0) { - ++errcnt; - ferrout("pr: cannot get time of day, %s\n", -strerror(errno)); - return(NULL); - } - curtime = tv.tv_sec; + curtime = time(NULL); timeptr = localtime(); } else { if (fstat(fileno(inf), ) < 0) { Index: usr.sbin/arp/arp.c === RCS file: /cvs/src/usr.sbin/arp/arp.c,v retrieving revision 1.79 diff -u -p -r1.79 arp.c --- usr.sbin/arp/arp.c 19 Apr 2017 05:36:12 -
fc-list.1 bug
$ diff -u fc-list.1.orig fc-list.1 --- fc-list.1.orig Fri Dec 22 18:47:19 2017 +++ fc-list.1 Fri Dec 22 18:54:24 2017 @@ -69,7 +69,7 @@ \fBfc-scan\fR(1) .PP The fontconfig user's guide, in HTML format: -\fI/usr/share/doc/fontconfig/fontconfig-user.html\fR\&. +\fI/usr/X11R6/share/doc/fontconfig/fontconfig-user.html\fR\&. .SH "AUTHOR" .PP This manual page was written by Keith Packard correct path to fontconfig-user.html
Re: uniq: add -i option
Hi Theo, Theo Buehler wrote on Thu, Dec 21, 2017 at 11:57:12PM +0100: > Ingo Schwarze wrote: >> So i really don't feel like adding a BUGS section, but instead i >> think documenting that -i is intended as an ASCII-only feature is >> the way to go. > Yes, sounds reasonable. Thanks for checking! Changes to DESCRIPTION and ENVIRONMENT committed. >> While here, profit from the opportunity to mention that uniq(1) is >> intended to work on the level of codepoints, not on the level of >> fully combined characters. > ok, I think that's an improvement. Thanks. I didn't commit the CAVEATS section because deraadt@ convincingly pointed out that it was quite hard to understand. Fully understanding a run-of-the-mill section 1 manual page must not require knowledge about Unicode terms like "normalization forms" and "canonical equivalence". Besides, the way our base system utilities define "string equality" for strings that may be either ASCII or UTF-8 is not specific to uniq(1). So i'll probably document that in another, central place, quite possibly in locale(1) because that's where LC_CTYPE is defined, so people are likely to look there for the gory details of UTF-8 handling, and also because using full Unicode terminology in that place is less disruptive than in an innocent manual line uniq(1). Yours, Ingo
Re: rasops(9): Remove dead assignment
> Date: Fri, 22 Dec 2017 23:35:47 +0100 > From: Frederic Cambus> > Hi tech@, > > Now that we call rasops_putchar_rotated(), we don't need ri anymore. > > Comments? OK? ok kettenis@ > Index: sys/dev/rasops/rasops.c > === > RCS file: /cvs/src/sys/dev/rasops/rasops.c,v > retrieving revision 1.48 > diff -u -p -r1.48 rasops.c > --- sys/dev/rasops/rasops.c 22 Aug 2017 12:24:45 - 1.48 > +++ sys/dev/rasops/rasops.c 22 Dec 2017 21:08:39 - > @@ -1265,11 +1265,8 @@ rasops_putchar_rotated(void *cookie, int > int > rasops_erasecols_rotated(void *cookie, int row, int col, int num, long attr) > { > - struct rasops_info *ri; > int i; > int rc; > - > - ri = (struct rasops_info *)cookie; > > for (i = col; i < col + num; i++) { > rc = rasops_putchar_rotated(cookie, row, i, ' ', attr); > >
Re: release(8): join exports
On 2017/12/22 22:12, Anton Lindqvist wrote: > Hi, > Since export accepts several variables, put them on a single line. > > Comments? OK? > > Index: release.8 > === > RCS file: /cvs/src/share/man/man8/release.8,v > retrieving revision 1.89 > diff -u -p -r1.89 release.8 > --- release.8 5 Jun 2017 22:27:58 - 1.89 > +++ release.8 22 Dec 2017 21:07:34 - > @@ -202,7 +202,7 @@ is also used and must not be configured. > .Pp > Make the release and check the contents of the release tarballs: > .Bd -literal -offset indent > -# export DESTDIR=your-destdir; export RELEASEDIR=your-releasedir > +# export DESTDIR=your-destdir RELEASEDIR=your-releasedir > # cd /usr/src/etc && make release > # cd /usr/src/distrib/sets && sh checkflist > # unset RELEASEDIR DESTDIR > @@ -240,7 +240,7 @@ will be removed. > .Pp > The steps to build and validate the Xenocara release are: > .Bd -literal -offset indent > -# export DESTDIR=your-destdir; export RELEASEDIR=your-releasedir > +# export DESTDIR=your-destdir RELEASEDIR=your-releasedir > # make release > # make checkdist > # unset RELEASEDIR DESTDIR > @@ -263,8 +263,7 @@ and > are suitable for installs without network connectivity. > They contain the tarballs and ports built in the previous steps. > .Bd -literal -offset indent > -# export RELDIR=your-releasedir > -# export RELXDIR=your-xenocara-releasedir > +# export RELDIR=your-releasedir RELXDIR=your-xenocara-releasedir > # cd /usr/src/distrib/$(machine)/iso && make > # make install > .Ed > If anything I'd go in the opposite direction and split the first two onto separate lines, they need editing to use the actual directory names anyway and it's easier to remove characters from the end of the line than make changes in the middle.. Not that it matters much, most people who do this regularly just script it afaik.
rasops(9): Remove dead assignment
Hi tech@, Now that we call rasops_putchar_rotated(), we don't need ri anymore. Comments? OK? Index: sys/dev/rasops/rasops.c === RCS file: /cvs/src/sys/dev/rasops/rasops.c,v retrieving revision 1.48 diff -u -p -r1.48 rasops.c --- sys/dev/rasops/rasops.c 22 Aug 2017 12:24:45 - 1.48 +++ sys/dev/rasops/rasops.c 22 Dec 2017 21:08:39 - @@ -1265,11 +1265,8 @@ rasops_putchar_rotated(void *cookie, int int rasops_erasecols_rotated(void *cookie, int row, int col, int num, long attr) { - struct rasops_info *ri; int i; int rc; - - ri = (struct rasops_info *)cookie; for (i = col; i < col + num; i++) { rc = rasops_putchar_rotated(cookie, row, i, ' ', attr);
Re: less(1): `!' command
On 2017/12/22 19:47, Nicholas Marriott wrote: > I don't think we should bring ! back. > > I wanted to remove v and | (and some other stuff) shortly afterwards, but > several people objected. > > I did suggest having a lightweight less in base for most people and adding > the full upstream less to ports for the stuff we don't want to maintain > (like we do for eg libevent) but other people didn't like that idea. less(1) can already be made more lightweight by setting LESSSECURE=1. (I quite like this even without the reduced pledge, my biggest annoyance with less is when I accidentally press 'v'). Any opinions on switching the default? Index: main.c === RCS file: /cvs/src/usr.bin/less/main.c,v retrieving revision 1.35 diff -u -p -u -1 -2 -r1.35 main.c --- main.c 17 Sep 2016 15:06:41 - 1.35 +++ main.c 22 Dec 2017 22:19:04 - @@ -87,17 +87,17 @@ main(int argc, char *argv[]) - secure = 0; + secure = 1; s = lgetenv("LESSSECURE"); - if (s != NULL && *s != '\0') - secure = 1; + if (s != NULL && strcmp(s, "0") == 0) + secure = 0; if (secure) { if (pledge("stdio rpath wpath tty", NULL) == -1) { perror("pledge"); exit(1); } } else { if (pledge("stdio rpath wpath cpath fattr proc exec tty", NULL) == -1) { perror("pledge"); exit(1); } } Index: less.1 === RCS file: /cvs/src/usr.bin/less/less.1,v retrieving revision 1.52 diff -u -p -r1.52 less.1 --- less.1 24 Oct 2016 13:46:58 - 1.52 +++ less.1 22 Dec 2017 22:17:28 - @@ -1674,9 +1674,7 @@ differences in invocation syntax, the .Ev LESSEDIT variable can be changed to modify this default. .Sh SECURITY -When the environment variable -.Ev LESSSECURE -is set to 1, +Normally, .Nm runs in a "secure" mode. This means these features are disabled: @@ -1698,6 +1696,10 @@ Metacharacters in filenames, such as "*" .It " " Filename completion (TAB, ^L). .El +.Pp +To enable these features, set the environment variable +.Ev LESSSECURE +to 0. .Sh COMPATIBILITY WITH MORE If the environment variable .Ev LESS_IS_MORE
Re: release(8): join exports
> Date: Fri, 22 Dec 2017 22:12:19 +0100 > From: Anton Lindqvist> > Hi, > Since export accepts several variables, put them on a single line. > > Comments? OK? Feels like unnecessary chrun to me. The exports are already on a single line, so there is no benefit for copy and pasting them into a shell. > Index: release.8 > === > RCS file: /cvs/src/share/man/man8/release.8,v > retrieving revision 1.89 > diff -u -p -r1.89 release.8 > --- release.8 5 Jun 2017 22:27:58 - 1.89 > +++ release.8 22 Dec 2017 21:07:34 - > @@ -202,7 +202,7 @@ is also used and must not be configured. > .Pp > Make the release and check the contents of the release tarballs: > .Bd -literal -offset indent > -# export DESTDIR=your-destdir; export RELEASEDIR=your-releasedir > +# export DESTDIR=your-destdir RELEASEDIR=your-releasedir > # cd /usr/src/etc && make release > # cd /usr/src/distrib/sets && sh checkflist > # unset RELEASEDIR DESTDIR > @@ -240,7 +240,7 @@ will be removed. > .Pp > The steps to build and validate the Xenocara release are: > .Bd -literal -offset indent > -# export DESTDIR=your-destdir; export RELEASEDIR=your-releasedir > +# export DESTDIR=your-destdir RELEASEDIR=your-releasedir > # make release > # make checkdist > # unset RELEASEDIR DESTDIR > @@ -263,8 +263,7 @@ and > are suitable for installs without network connectivity. > They contain the tarballs and ports built in the previous steps. > .Bd -literal -offset indent > -# export RELDIR=your-releasedir > -# export RELXDIR=your-xenocara-releasedir > +# export RELDIR=your-releasedir RELXDIR=your-xenocara-releasedir > # cd /usr/src/distrib/$(machine)/iso && make > # make install > .Ed > >
release(8): join exports
Hi, Since export accepts several variables, put them on a single line. Comments? OK? Index: release.8 === RCS file: /cvs/src/share/man/man8/release.8,v retrieving revision 1.89 diff -u -p -r1.89 release.8 --- release.8 5 Jun 2017 22:27:58 - 1.89 +++ release.8 22 Dec 2017 21:07:34 - @@ -202,7 +202,7 @@ is also used and must not be configured. .Pp Make the release and check the contents of the release tarballs: .Bd -literal -offset indent -# export DESTDIR=your-destdir; export RELEASEDIR=your-releasedir +# export DESTDIR=your-destdir RELEASEDIR=your-releasedir # cd /usr/src/etc && make release # cd /usr/src/distrib/sets && sh checkflist # unset RELEASEDIR DESTDIR @@ -240,7 +240,7 @@ will be removed. .Pp The steps to build and validate the Xenocara release are: .Bd -literal -offset indent -# export DESTDIR=your-destdir; export RELEASEDIR=your-releasedir +# export DESTDIR=your-destdir RELEASEDIR=your-releasedir # make release # make checkdist # unset RELEASEDIR DESTDIR @@ -263,8 +263,7 @@ and are suitable for installs without network connectivity. They contain the tarballs and ports built in the previous steps. .Bd -literal -offset indent -# export RELDIR=your-releasedir -# export RELXDIR=your-xenocara-releasedir +# export RELDIR=your-releasedir RELXDIR=your-xenocara-releasedir # cd /usr/src/distrib/$(machine)/iso && make # make install .Ed
Re: less(1): `!' command
I don't think we should bring ! back. I wanted to remove v and | (and some other stuff) shortly afterwards, but several people objected. I did suggest having a lightweight less in base for most people and adding the full upstream less to ports for the stuff we don't want to maintain (like we do for eg libevent) but other people didn't like that idea. On 17 December 2017 at 15:48, kshewrote: > On Sat, 16 Dec 2017 21:52:44 +, Theo de Raadt wrote: > > > On Sat, 16 Dec 2017 19:39:27 +, Theo de Raadt wrote: > > > > > On Sat, 16 Dec 2017 18:13:16 +, Jiri B wrote: > > > > > > On Sat, Dec 16, 2017 at 04:55:44PM +, kshe wrote: > > > > > > > Hi, > > > > > > > > > > > > > > Would a patch to bring back the `!' command to less(1) be > accepted? The > > > > > > > commit message for its removal explains that ^Z should be used > instead, > > > > > > > but that obviously does not work if less(1) is run from > something else > > > > > > > than an interactive shell, for example when reading manual > pages from a > > > > > > > vi(1) instance spawned directly by `xterm -e vi' in a window > manager or > > > > > > > by `neww vi' in a tmux(1) session. > > > > > > > > > > > > Why should less be able to spawn another programs? This would > undermine > > > > > > all pledge work. > > > > > > > > > > Because of at least `v' and `|', less(1) already is able to invoke > > > > > arbitrary programs, and accordingly needs the "proc exec" promise, > so > > > > > bringing `!' back would not change anything from a security > perspective > > > > > (otherwise, I would obviously not have made such a proposition). > > > > > > > > > > In fact, technically, what I want to do is still currently > possible: > > > > > from any less(1) instance, one may use `v' to invoke vi(1), and > then use > > > > > vi(1)'s own `!' command as desired. So the functionality of `!' is > > > > > still there; it was only made more difficult to reach for no > apparent > > > > > reason. > > > > > > > > No apparent reason? > > > > > > > > Good you have an opinion. I have a different opinion: We should look > > > > for rarely used functionality and gut it. > > > > > > I completely agree, and I also completely agree with the rest of what > > > you said. However, in this particular case, the functionality of `!' > is > > > still fully (albeit indirectly) accessible, as shown above, and this is > > > why its deletion, when not immediately followed by that of `|' and `v', > > > made little sense for me. > > > > Oh, so you don't agree. Or do you. I can't tell. You haven't made up > > your mind enough to have a final position? > > In the case of less(1), the underlying functionality of `!' (invoking > arbitrary programs) has not been removed at all, as `!' itself was only > one way amongst others of doing that. Therefore, I would have prefered > that such an endeavour be conducted in steps at least as large as a > pledge(2) category. You may say this is absolutist, but, in the end, > users might actually be more inclined to accept such removals if they > come with, and thus are justified by, a real and immediate security > benefit, like stricter pledge(2) promises, rather than some vague > theoretical explanation about the global state of their software > environment. > > > [...] > > > > > May I go ahead and prepare a patch to remove "proc exec" entirely? > > > > Sure you could try, and see who freaks out. Exactly what the plan was > > all along. > > The minimal diff below does that. If it is accepted, further cleanups > would need to follow (in particular, removing a few unused variables and > functions), and of course the manual would also need some adjustments. > > Index: cmd.h > === > RCS file: /cvs/src/usr.bin/less/cmd.h,v > retrieving revision 1.10 > diff -u -p -r1.10 cmd.h > --- cmd.h 6 Nov 2015 15:58:01 - 1.10 > +++ cmd.h 17 Dec 2017 12:23:00 - > @@ -42,12 +42,12 @@ > #defineA_FF_LINE 29 > #defineA_BF_LINE 30 > #defineA_VERSION 31 > -#defineA_VISUAL32 > +/* 32 unused */ > #defineA_F_WINDOW 33 > #defineA_B_WINDOW 34 > #defineA_F_BRACKET 35 > #defineA_B_BRACKET 36 > -#defineA_PIPE 37 > +/* 37 unused */ > #defineA_INDEX_FILE38 > #defineA_UNDO_SEARCH 39 > #defineA_FF_SCREEN 40 > Index: command.c > === > RCS file: /cvs/src/usr.bin/less/command.c,v > retrieving revision 1.31 > diff -u -p -r1.31 command.c > --- command.c 12 Jan 2017 20:32:01 - 1.31 > +++ command.c 17 Dec 2017 12:23:00 - > @@ -241,12 +241,6 @@ exec_mca(void) > /* If tag structure is loaded then clean it up. */ >
Re: realpath(3): some buffer overflows and conformance issues
On Fri, 22 Dec 2017 12:08:58 +0100, =?UTF-8?Q?Jan_Kokem=c3=bcller?= wrote: > I've found some buffer overflows in realpath(3). They are limited to > just two bytes though (one after the 'left' buffer and one before > 'symlink'), so the impact is minimal. > > Similar bugs in FreeBSD: > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219154 > > > Here is a list of issues: > > - The statement "left_len -= s - left;" does not take the slash into >account if one was found. This results in the invariant >"left[left_len] == '\0'" being violated (and possible buffer >overflows). The patch replaces the variable "s" with a size_t >"next_token_len" for more clarity. > > - "slen" from readlink(2) can be 0 when encountering empty symlinks. >Then, further down, "symlink[slen - 1]" underflows the buffer. When >slen == 0, realpath(3) should probably return ENOENT >(http://austingroupbugs.net/view.php?id=825, >https://lwn.net/Articles/551224/). > > - return ENOENT if path is NULL (POSIX requires this) > > - The condition "resolved_len >= PATH_MAX" cannot be true. > > - Similarly, "s - left >= sizeof(next_token)" cannot be true, as long >as "sizeof(next_token) >= sizeof(left)". > > - Return ENAMETOOLONG when a resolved symlink from readlink(2) is too >long for the symlink buffer (instead of just truncating it). > > - "resolved_len > 1" below the call to readlink(2) is always true as >"strlcat(resolved, next_token, PATH_MAX);" always results in a string >of length > 1. Also, "resolved[resolved_len - 1] = '\0';" is not >needed; there can never be a trailing slash here. > > > Here is a patch. I must admit that I haven't tested this on OpenBSD but > compiled the OpenBSD version on FreeBSD. I have left in some asserts to > describe loop invariants. Those can be removed I guess. I've just > noticed the realpath copy in 'libexec/ld.so/dl_realpath.c' that should > be fixed similarly. The patch looks correct to me. We can either remove the asserts() or simply define NDEBUG before including assert.h. Anyone else want to OK this? - todd
Re: armv7 a few tc_counter_mask fixes
On Mon, Sep 18, 2017 at 08:53:43PM +0300, Artturi Alm wrote: > On Mon, Sep 18, 2017 at 03:41:56PM +0300, Artturi Alm wrote: > > On Mon, Sep 18, 2017 at 11:19:09AM +0100, Stuart Henderson wrote: > > > On 2017/09/18 04:28, Artturi Alm wrote: > > > > Do i really need to reference datasheets, or would someone explain to me > > > > the value of this MSB robbing? > > > > > > I think, if you're proposing a change, you should explain why that > > > change should be made, rather than asking others to defend the current > > > situation.. > > > > > > > guess i wasn't clear enough. of the 3 timers two are 64bit timers, of them > > agtimer doesn't even support reading just 32bit, nor any of them do just > > 31bits as claimed by the timercounter mask. > > so all of them act opposite to what's written in sys/timetc.h, the last bit > > won't be constant with these. > > > > amptimer's low register does have full 32bits, if it didn't, i doubt this > > function could exist: > > /sys/arch/arm/cortex/amptimer.c: > > 128 uint64_t > > 129 amptimer_readcnt64(struct amptimer_softc *sc) > > 130 { > > 131 uint32_t high0, high1, low; > > 132 bus_space_tag_t iot = sc->sc_iot; > > 133 bus_space_handle_t ioh = sc->sc_ioh; > > 134 > > 135 do { > > 136 high0 = bus_space_read_4(iot, ioh, GTIMER_CNT_HIGH); > > 137 low = bus_space_read_4(iot, ioh, GTIMER_CNT_LOW); > > 138 high1 = bus_space_read_4(iot, ioh, GTIMER_CNT_HIGH); > > 139 } while (high0 != high1); > > 140 > > 141 return uint64_t)high1) << 32) | low); > > 142 } > > > > if you google for "swpu223g" you'll find omap3430 technical reference manual > > pdf, in it you can find the description of gptimer's TCRR register, at > > page 2600, also it _will_ count beyond 0x7fff. > > > > you can find reference to gptimer(missed replace) from amptimer.c, i guess > > amptimer was where agtimer got it from, so maybe just an bad copy-paste. > > > > i meant to write "(missed replace?)" above, as i'm not sure, > but now i think i know who i should have cc'ed initially. > i'm guessing the chain has gone something like this: > macppc||socppc->beagle's gptimer->panda's amptimer->agtimer->arm64 agtimer > > drahn@, would you help me out a bit here? do you know/remember about > these enough, to ok what i've suggested(+same for arm64 agtimer) to anyone > who could pick this up? > > -Artturi > ping? would you, just for a bit? -Artturi
Re: armv7/imx: freezing fec
On Sun, Dec 10, 2017 at 07:05:11PM +0100, Mark Kettenis wrote: > > Date: Sun, 10 Dec 2017 19:03:41 +0200 > > From: Artturi Alm> > > > On Wed, Nov 29, 2017 at 11:45:51AM +0200, Artturi Alm wrote: > > > Hi, > > > > > > > > > there's more work to be done for fec, but this will allow changing > > > changing address/ifconfig up etc. without the freeze/kernel hangup > > > preventing ie. autoinstall i've reported to bugs@. > > > > > > i didn't see these while loops being done in nbsd if_enet, where i think > > > fec originates from. > > > > > > -Artturi > > > > > > > Ping? should I minimize the diff i sent earlier, or? > > Does this fix any real problems? If not, just drop it. > Pong; i dropped imx, and replugged a64pine in it's place, but here's a lesser version untested of a diff that fixed the bug for me. -Artturi diff --git a/sys/arch/armv7/imx/if_fec.c b/sys/arch/armv7/imx/if_fec.c index 899c1904144..461ab74041d 100644 --- a/sys/arch/armv7/imx/if_fec.c +++ b/sys/arch/armv7/imx/if_fec.c @@ -181,6 +181,8 @@ #define ENET_TXD_INT (1 << 30) #endif +#defineENET_MII_TIMEOUT10 /* 5sec: --loop_cnt { delay(5); } */ + /* * Bus dma allocation structure used by * fec_dma_malloc and fec_dma_free. @@ -339,7 +341,6 @@ fec_attach(struct device *parent, struct device *self, void *aux) /* reset the controller */ HSET4(sc, ENET_ECR, ENET_ECR_RESET); - while(HREAD4(sc, ENET_ECR) & ENET_ECR_RESET); HWRITE4(sc, ENET_EIMR, 0); HWRITE4(sc, ENET_EIR, 0x); @@ -604,7 +605,6 @@ fec_init(struct fec_softc *sc) /* reset the controller */ HSET4(sc, ENET_ECR, ENET_ECR_RESET); - while(HREAD4(sc, ENET_ECR) & ENET_ECR_RESET); /* set hw address */ HWRITE4(sc, ENET_PALR, @@ -616,7 +616,8 @@ fec_init(struct fec_softc *sc) (sc->sc_ac.ac_enaddr[4] << 24) | (sc->sc_ac.ac_enaddr[5] << 16)); - /* clear outstanding interrupts */ + /* mask and clear all (possibly outstanding) interrupts */ + HWRITE4(sc, ENET_EIMR, 0); HWRITE4(sc, ENET_EIR, 0x); /* set max receive buffer size, 3-0 bits always zero for alignment */ @@ -692,6 +693,9 @@ fec_stop(struct fec_softc *sc) { struct ifnet *ifp = >sc_ac.ac_if; + /* stop the controller, will be reset on _init() */ + HCLR4(sc, ENET_ECR, ENET_ECR_ETHEREN); + /* * Mark the interface down and cancel the watchdog timer. */ @@ -700,10 +704,6 @@ fec_stop(struct fec_softc *sc) ifq_clr_oactive(>if_snd); timeout_del(>sc_tick); - - /* reset the controller */ - HSET4(sc, ENET_ECR, ENET_ECR_RESET); - while(HREAD4(sc, ENET_ECR) & ENET_ECR_RESET); } void @@ -996,6 +996,7 @@ fec_miibus_readreg(struct device *dev, int phy, int reg) { int r = 0; struct fec_softc *sc = (struct fec_softc *)dev; + u_int timo = ENET_MII_TIMEOUT; HSET4(sc, ENET_EIR, ENET_EIR_MII); @@ -1003,7 +1004,8 @@ fec_miibus_readreg(struct device *dev, int phy, int reg) ENET_MMFR_ST | ENET_MMFR_OP_RD | ENET_MMFR_TA | phy << ENET_MMFR_PA_SHIFT | reg << ENET_MMFR_RA_SHIFT); - while(!(HREAD4(sc, ENET_EIR) & ENET_EIR_MII)); + while (!(HREAD4(sc, ENET_EIR) & ENET_EIR_MII) && --timo) + delay(5); r = bus_space_read_4(sc->sc_iot, sc->sc_ioh, ENET_MMFR); @@ -1014,6 +1016,7 @@ void fec_miibus_writereg(struct device *dev, int phy, int reg, int val) { struct fec_softc *sc = (struct fec_softc *)dev; + u_int timo = ENET_MII_TIMEOUT; HSET4(sc, ENET_EIR, ENET_EIR_MII); @@ -1022,7 +1025,8 @@ fec_miibus_writereg(struct device *dev, int phy, int reg, int val) phy << ENET_MMFR_PA_SHIFT | reg << ENET_MMFR_RA_SHIFT | (val & 0x)); - while(!(HREAD4(sc, ENET_EIR) & ENET_EIR_MII)); + while (!(HREAD4(sc, ENET_EIR) & ENET_EIR_MII) && --timo) + delay(5); return; }
Re: pf divert raw sockets
On Fri, Dec 15, 2017 at 03:10:49PM +0100, Alexander Bluhm wrote: > Hi, > > With pf divert raw sockets behave differently than TCP or UDP. > > TCP and UDP first make an exact match without divert in_pcbhashlookup(). > Then they do the divert-to match in in_pcblookup_listen(). > > This diff makes it consistent. Search for bound and connected > sockets in raw_input() also if diverted, only use unconnected sockets > with divert-to. > > ok? Anyone? > Index: netinet/raw_ip.c > === > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/raw_ip.c,v > retrieving revision 1.108 > diff -u -p -r1.108 raw_ip.c > --- netinet/raw_ip.c 4 Dec 2017 13:40:34 - 1.108 > +++ netinet/raw_ip.c 15 Dec 2017 12:59:29 - > @@ -121,7 +121,7 @@ rip_input(struct mbuf **mp, int *offp, i > struct mbuf *m = *mp; > struct ip *ip = mtod(m, struct ip *); > struct inpcb *inp, *last = NULL; > - struct in_addr *key; > + struct in_addr *key = NULL; > struct mbuf *opts = NULL; > struct counters_ref ref; > uint64_t *counters; > @@ -129,7 +129,6 @@ rip_input(struct mbuf **mp, int *offp, i > KASSERT(af == AF_INET); > > ripsrc.sin_addr = ip->ip_src; > - key = >ip_dst; > #if NPF > 0 > if (m->m_pkthdr.pf.flags & PF_TAG_DIVERTED) { > struct pf_divert *divert; > @@ -163,7 +162,10 @@ rip_input(struct mbuf **mp, int *offp, i > if (inp->inp_ip.ip_p && inp->inp_ip.ip_p != ip->ip_p) > continue; > if (inp->inp_laddr.s_addr && > - inp->inp_laddr.s_addr != key->s_addr) > + inp->inp_laddr.s_addr != ip->ip_dst.s_addr && > + /* divert-to matches on bound but unconnected socket */ > + (key == NULL || inp->inp_faddr.s_addr || > + inp->inp_laddr.s_addr != key->s_addr)) > continue; > if (inp->inp_faddr.s_addr && > inp->inp_faddr.s_addr != ip->ip_src.s_addr) > Index: netinet6/raw_ip6.c > === > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/raw_ip6.c,v > retrieving revision 1.125 > diff -u -p -r1.125 raw_ip6.c > --- netinet6/raw_ip6.c4 Dec 2017 13:40:35 - 1.125 > +++ netinet6/raw_ip6.c15 Dec 2017 12:59:29 - > @@ -122,7 +122,7 @@ rip6_input(struct mbuf **mp, int *offp, > struct ip6_hdr *ip6 = mtod(m, struct ip6_hdr *); > struct inpcb *in6p; > struct inpcb *last = NULL; > - struct in6_addr *key; > + struct in6_addr *key = NULL; > struct sockaddr_in6 rip6src; > struct mbuf *opts = NULL; > > @@ -137,7 +137,6 @@ rip6_input(struct mbuf **mp, int *offp, > /* KAME hack: recover scopeid */ > in6_recoverscope(, >ip6_src); > > - key = >ip6_dst; > #if NPF > 0 > if (m->m_pkthdr.pf.flags & PF_TAG_DIVERTED) { > struct pf_divert *divert; > @@ -167,7 +166,10 @@ rip6_input(struct mbuf **mp, int *offp, > in6p->inp_ipv6.ip6_nxt != proto) > continue; > if (!IN6_IS_ADDR_UNSPECIFIED(>inp_laddr6) && > - !IN6_ARE_ADDR_EQUAL(>inp_laddr6, key)) > + !IN6_ARE_ADDR_EQUAL(>inp_laddr6, >ip6_dst) && > + /* divert-to matches on bound but unconnected socket */ > + (key == NULL || !IN6_IS_ADDR_UNSPECIFIED(>inp_faddr6) > + || !IN6_ARE_ADDR_EQUAL(>inp_laddr6, key))) > continue; > if (!IN6_IS_ADDR_UNSPECIFIED(>inp_faddr6) && > !IN6_ARE_ADDR_EQUAL(>inp_faddr6, >ip6_src))
pf state key link inp
Hi, There is a corner case where linking the inp to the state key does not work in pf. The function pf_inp_link() takes the state key from the mbuf and not the one we have just found. Introduce a new function pf_state_key_link_inpcb() that does exactly that together with some sanity checks. I have regress tests that can trigger cases where this is relevant. ok? bluhm Index: net/pf.c === RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v retrieving revision 1.1050 diff -u -p -r1.1050 pf.c --- net/pf.c4 Dec 2017 15:13:12 - 1.1050 +++ net/pf.c22 Dec 2017 14:21:09 - @@ -249,6 +249,8 @@ void pf_state_key_link(struct pf_stat struct pf_state_key *); voidpf_inpcb_unlink_state_key(struct inpcb *); voidpf_state_key_unlink_reverse(struct pf_state_key *); +voidpf_state_key_link_inpcb(struct pf_state_key *, + struct inpcb *); #if NPFLOG > 0 voidpf_log_matches(struct pf_pdesc *, struct pf_rule *, @@ -1085,8 +1087,9 @@ pf_find_state(struct pfi_kif *kif, struc if (dir == PF_OUT && pkt_sk && pf_compare_state_keys(pkt_sk, sk, kif, dir) == 0) pf_state_key_link(sk, pkt_sk); - else if (dir == PF_OUT) - pf_inp_link(m, m->m_pkthdr.pf.inp); + else if (dir == PF_OUT && m->m_pkthdr.pf.inp && + !m->m_pkthdr.pf.inp->inp_pf_sk && !sk->inp) + pf_state_key_link_inpcb(sk, m->m_pkthdr.pf.inp); } /* remove firewall data from outbound packet */ @@ -7259,6 +7262,15 @@ void pf_pkt_state_key_ref(struct mbuf *m) { pf_state_key_ref(m->m_pkthdr.pf.statekey); +} + +void +pf_state_key_link_inpcb(struct pf_state_key *sk, struct inpcb *inp) +{ + KASSERT(sk->inp == NULL); + sk->inp = inp; + KASSERT(inp->inp_pf_sk == NULL); + inp->inp_pf_sk = pf_state_key_ref(sk); } void
llvm: patch-update to release 5.0.1
Hi, I would like to update LLVM, including clang, lld and lldb, to the patch release version 5.0.1. They typically only do bugfixing, which means that the diff to 5.0.0 is "only" about 3500 lines. I don't expect any big fallout, but I would appreciate if someone could do a ports run with this change. I will take care of src/x11 on amd64. Update procedure is simply running $ cd /usr/src/gnu/usr.bin/clang $ make obj $ make clean $ make -j8 $ make install Feedback appreciated! Patrick diff --git a/gnu/llvm/CMakeLists.txt b/gnu/llvm/CMakeLists.txt index 8c0f5114513..960febf6007 100644 --- a/gnu/llvm/CMakeLists.txt +++ b/gnu/llvm/CMakeLists.txt @@ -26,7 +26,7 @@ if(NOT DEFINED LLVM_VERSION_MINOR) set(LLVM_VERSION_MINOR 0) endif() if(NOT DEFINED LLVM_VERSION_PATCH) - set(LLVM_VERSION_PATCH 0) + set(LLVM_VERSION_PATCH 1) endif() if(NOT DEFINED LLVM_VERSION_SUFFIX) set(LLVM_VERSION_SUFFIX "") @@ -208,10 +208,6 @@ include(VersionFromVCS) option(LLVM_APPEND_VC_REV "Embed the version control system revision id in LLVM" ON) -if( LLVM_APPEND_VC_REV ) - add_version_info_from_vcs(PACKAGE_VERSION) -endif() - set(PACKAGE_NAME LLVM) set(PACKAGE_STRING "${PACKAGE_NAME} ${PACKAGE_VERSION}") set(PACKAGE_BUGREPORT "http://llvm.org/bugs/;) diff --git a/gnu/llvm/docs/CMake.rst b/gnu/llvm/docs/CMake.rst index bf97e917315..b6ebf37adc9 100644 --- a/gnu/llvm/docs/CMake.rst +++ b/gnu/llvm/docs/CMake.rst @@ -248,9 +248,10 @@ LLVM-specific variables **LLVM_APPEND_VC_REV**:BOOL Embed version control revision info (svn revision number or Git revision id). - This is used among other things in the LLVM version string (stored in the - PACKAGE_VERSION macro). For this to work cmake must be invoked before the - build. Defaults to ON. + The version info is provided by the ``LLVM_REVISION`` macro in + ``llvm/include/llvm/Support/VCSRevision.h``. Developers using git who don't + need revision info can disable this option to avoid re-linking most binaries + after a branch switch. Defaults to ON. **LLVM_ENABLE_THREADS**:BOOL Build with threads support, if available. Defaults to ON. diff --git a/gnu/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h b/gnu/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h index 0b07fe9aa23..9bbda718aca 100644 --- a/gnu/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h +++ b/gnu/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h @@ -652,6 +652,12 @@ public: auto GTI = gep_type_begin(PointeeType, Operands); Type *TargetType; + +// Handle the case where the GEP instruction has a single operand, +// the basis, therefore TargetType is a nullptr. +if (Operands.empty()) + return !BaseGV ? TTI::TCC_Free : TTI::TCC_Basic; + for (auto I = Operands.begin(); I != Operands.end(); ++I, ++GTI) { TargetType = GTI.getIndexedType(); // We assume that the cost of Scalar GEP with constant index and the diff --git a/gnu/llvm/include/llvm/CodeGen/MachineRegisterInfo.h b/gnu/llvm/include/llvm/CodeGen/MachineRegisterInfo.h index 8347f00cbc7..5ef0ac90e3c 100644 --- a/gnu/llvm/include/llvm/CodeGen/MachineRegisterInfo.h +++ b/gnu/llvm/include/llvm/CodeGen/MachineRegisterInfo.h @@ -807,6 +807,14 @@ public: return getReservedRegs().test(PhysReg); } + /// Returns true when the given register unit is considered reserved. + /// + /// Register units are considered reserved when for at least one of their + /// root registers, the root register and all super registers are reserved. + /// This currently iterates the register hierarchy and may be slower than + /// expected. + bool isReservedRegUnit(unsigned Unit) const; + /// isAllocatable - Returns true when PhysReg belongs to an allocatable /// register class and it hasn't been reserved. /// diff --git a/gnu/llvm/include/llvm/IR/AutoUpgrade.h b/gnu/llvm/include/llvm/IR/AutoUpgrade.h index b42a3d3ad95..3f406f0cf19 100644 --- a/gnu/llvm/include/llvm/IR/AutoUpgrade.h +++ b/gnu/llvm/include/llvm/IR/AutoUpgrade.h @@ -51,6 +51,8 @@ namespace llvm { /// module is modified. bool UpgradeModuleFlags(Module ); + void UpgradeSectionAttributes(Module ); + /// If the given TBAA tag uses the scalar TBAA format, create a new node /// corresponding to the upgrade to the struct-path aware TBAA format. /// Otherwise return the \p TBAANode itself. diff --git a/gnu/llvm/include/llvm/Support/FormatVariadic.h b/gnu/llvm/include/llvm/Support/FormatVariadic.h index c1153e84dfb..408c6d8b2e0 100644 --- a/gnu/llvm/include/llvm/Support/FormatVariadic.h +++ b/gnu/llvm/include/llvm/Support/FormatVariadic.h @@ -94,6 +94,15 @@ public: Adapters.reserve(ParamCount); } + formatv_object_base(formatv_object_base const ) = delete; + + formatv_object_base(formatv_object_base &) + : Fmt(std::move(rhs.Fmt)), +Adapters(), // Adapters are initialized by formatv_object +Replacements(std::move(rhs.Replacements)) { +
realpath(3): some buffer overflows and conformance issues
Hi, I've found some buffer overflows in realpath(3). They are limited to just two bytes though (one after the 'left' buffer and one before 'symlink'), so the impact is minimal. Similar bugs in FreeBSD: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219154 Here is a list of issues: - The statement "left_len -= s - left;" does not take the slash into account if one was found. This results in the invariant "left[left_len] == '\0'" being violated (and possible buffer overflows). The patch replaces the variable "s" with a size_t "next_token_len" for more clarity. - "slen" from readlink(2) can be 0 when encountering empty symlinks. Then, further down, "symlink[slen - 1]" underflows the buffer. When slen == 0, realpath(3) should probably return ENOENT (http://austingroupbugs.net/view.php?id=825, https://lwn.net/Articles/551224/). - return ENOENT if path is NULL (POSIX requires this) - The condition "resolved_len >= PATH_MAX" cannot be true. - Similarly, "s - left >= sizeof(next_token)" cannot be true, as long as "sizeof(next_token) >= sizeof(left)". - Return ENAMETOOLONG when a resolved symlink from readlink(2) is too long for the symlink buffer (instead of just truncating it). - "resolved_len > 1" below the call to readlink(2) is always true as "strlcat(resolved, next_token, PATH_MAX);" always results in a string of length > 1. Also, "resolved[resolved_len - 1] = '\0';" is not needed; there can never be a trailing slash here. Here is a patch. I must admit that I haven't tested this on OpenBSD but compiled the OpenBSD version on FreeBSD. I have left in some asserts to describe loop invariants. Those can be removed I guess. I've just noticed the realpath copy in 'libexec/ld.so/dl_realpath.c' that should be fixed similarly. diff --git lib/libc/stdlib/realpath.c lib/libc/stdlib/realpath.c index c691252..31f7120 100644 --- lib/libc/stdlib/realpath.c +++ lib/libc/stdlib/realpath.c @@ -27,6 +27,7 @@ * SUCH DAMAGE. */ +#include #include #include #include @@ -45,12 +46,18 @@ char * realpath(const char *path, char *resolved) { - char *p, *q, *s; - size_t left_len, resolved_len; + char *p, *q; + size_t left_len, resolved_len, next_token_len; unsigned symlinks; - int serrno, slen, mem_allocated; + int serrno, mem_allocated; + ssize_t slen; char left[PATH_MAX], next_token[PATH_MAX], symlink[PATH_MAX]; + if (path == NULL) { + errno = EINVAL; + return (NULL); + } + if (path[0] == '\0') { errno = ENOENT; return (NULL); @@ -85,7 +92,7 @@ realpath(const char *path, char *resolved) resolved_len = strlen(resolved); left_len = strlcpy(left, path, sizeof(left)); } - if (left_len >= sizeof(left) || resolved_len >= PATH_MAX) { + if (left_len >= sizeof(left)) { errno = ENAMETOOLONG; goto err; } @@ -94,21 +101,28 @@ realpath(const char *path, char *resolved) * Iterate over path components in `left'. */ while (left_len != 0) { + assert(left[left_len] == '\0'); + /* * Extract the next path component and adjust `left' * and its length. */ p = strchr(left, '/'); - s = p ? p : left + left_len; - if (s - left >= sizeof(next_token)) { - errno = ENAMETOOLONG; - goto err; + + assert(sizeof(next_token) >= sizeof(left)); + + next_token_len = p ? (size_t) (p - left) : left_len; + memcpy(next_token, left, next_token_len); + next_token[next_token_len] = '\0'; + + if (p != NULL) { + left_len -= next_token_len + 1; + memmove(left, p + 1, left_len + 1); + } else { + left[0] = '\0'; + left_len = 0; } - memcpy(next_token, left, s - left); - next_token[s - left] = '\0'; - left_len -= s - left; - if (p != NULL) - memmove(left, s + 1, left_len + 1); + if (resolved[resolved_len - 1] != '/') { if (resolved_len + 1 >= PATH_MAX) { errno = ENAMETOOLONG; @@ -145,7 +159,7 @@ realpath(const char *path, char *resolved) errno = ENAMETOOLONG; goto err; } - slen = readlink(resolved, symlink, sizeof(symlink) - 1); + slen = readlink(resolved, symlink, sizeof(symlink)); if (slen < 0) { switch (errno) { case EINVAL: @@ -160,6 +174,12 @@ realpath(const char *path, char *resolved)