Re: systat(1) hostname

[Ted Unangst]

Theo de Raadt wrote:
> > On Wed, Oct 12, 2016 at 03:20:00PM +0200, Otto Moerbeek wrote:
> > > simple diff to show the hostname on the second line. OK?
> > 
> > OK bluhm@
> > 
> > > 
> > > BTW, batch mode doesn't function here as expected. Need to look into that,
> 
> I hoped this would look more like top(1) so I did it a different way.
> This does not work quite right for long hostnames, but then.. neither
> does top.

this truncates hostname to always display time.


Index: main.c
===
RCS file: /cvs/src/usr.bin/systat/main.c,v
retrieving revision 1.64
diff -u -p -r1.64 main.c
--- main.c  2 Jan 2016 15:02:05 -   1.64
+++ main.c  13 Oct 2016 04:13:45 -
@@ -67,6 +67,7 @@ int   ut, hz, stathz;
 charhostname[HOST_NAME_MAX+1];
 WINDOW  *wnd;
 intCMDLINE;
+char   hostbuf[26];
 char   timebuf[26];
 char   uloadbuf[TIMEPOS];
 
@@ -101,14 +102,20 @@ print_header(void)
tb_start();
 
if (!paused) {
+   char *ctim;
+
getloadavg(avenrun, sizeof(avenrun) / sizeof(avenrun[0]));
 
snprintf(uloadbuf, sizeof(uloadbuf),
"%5d usersLoad %.2f %.2f %.2f", 
ucount(), avenrun[0], avenrun[1], avenrun[2]);
 
+   gethostname(hostbuf, sizeof hostbuf);
+
time();
-   strlcpy(timebuf, ctime(), sizeof(timebuf));
+   ctim = ctime();
+   ctim[11+8] = '\0';
+   strlcpy(timebuf, ctim + 11, sizeof(timebuf));
}
 
if (num_disp && (start > 1 || end != num_disp))
@@ -120,7 +127,7 @@ print_header(void)
"%s %s", uloadbuf,
paused ? "PAUSED" : "");

-   snprintf(header, sizeof(header), "%-55s%s", tmpbuf, timebuf);
+   snprintf(header, sizeof(header), "%-45s%s %s", tmpbuf, hostbuf, 
timebuf);
 
if (rawmode)
printf("\n\n%s\n", header);

Re: /usr/src beforeinstall: make prereq as BUILDUSER

[Theo Buehler]

On Wed, Oct 12, 2016 at 10:30:26PM +0200, Theo Buehler wrote:
> Several people noticed that a few files from libstdc++-v3 in /usr/obj
> end up being owned by root, namely:
> 
> c++config.h gthr.h gthr-single.h gthr-posix.h gthr-tpf.h gthr-default.h
> 
> The problem is that they are regenerated by root when beforeinstall does
> 'make includes' directly from /usr/src/includes.  A cheap fix is to call
> make includes from the main Makefile, which was already fixed to
> de-escalate the prereq step.
> 
> Fixing that directly in gnu/lib/libstdc++-v3/Makefile would also be
> possible, but it seems to be quite a bit messier since one would have to
> touch quite a few targets to do the proper de-escalation.
> 
> With that, /usr/obj contains no root-owned files after 'make build'.
> 

Here's an improved version of the patch that doesn't interrupt
'make release' by asking for BUILDUSER's password because of trying
to do 'sh -c ${BUILDUSER}' as BUILDUSER.

Index: Makefile
===
RCS file: /var/cvs/src/Makefile,v
retrieving revision 1.129
diff -u -p -r1.129 Makefile
--- Makefile6 Oct 2016 18:56:17 -   1.129
+++ Makefile13 Oct 2016 00:19:13 -
@@ -57,7 +57,11 @@ includes:
 beforeinstall:
cd ${.CURDIR}/etc && exec ${MAKE} DESTDIR=${DESTDIR} distrib-dirs
cd ${.CURDIR}/etc && exec ${MAKE} DESTDIR=${DESTDIR} install-mtree
-   cd ${.CURDIR}/include && exec ${MAKE} includes
+   @if [[ `id -u` -ne 0 ]]; then \
+   cd ${.CURDIR}/include && exec ${MAKE} includes; \
+   else \
+   exec ${MAKE} includes; \
+   fi
 
 afterinstall:
 .ifndef NOMAN

Re: signify(1): make comments optional

[Ivan Markin]

Theo de Raadt:
> Ivan, you bothered to write the diff and reply quite a few times, but
> you waived all the concerns aside.

This was just my opinion on this. Nothing less, nothing more.

> Don't post diffs you don't want to defend.

Should I defend? I posted it because I want to share.

> You have a problem; when I ask question and you delete text and wave
> your hands in dismissal, it is not I that have a problem, but you.

There is no problem.

--
Ivan Markin

Re: signify(1): make comments optional

[Theo de Raadt]

> > > That is why you are trying to push changes into an established
> > > ecosystem, WITHOUT JUSTIFICATION.  Your only justification was
> > > "because I want to do so".
> > 
> > Read this tread carefully and you'll see that I don't *push* anything. I
> > don't consider one lonely patch on tech@ as pushing. tech@ is still the
> > upstream for signify(1). Why not leave it alone if you don't like to
> > merge it?
> > 
> > I'm not going to answer to another portion of pointless insults below.
> > Just wondering how can you produce such amount of it out nothing.
> 
> Ivan, you bothered to write the diff and reply quite a few times, but
> you waived all the concerns aside.
> 
> It's your problem not mine.

To put it simply:

Don't post diffs you don't want to defend.

You have a problem; when I ask question and you delete text and wave
your hands in dismissal, it is not I that have a problem, but you.

Re: signify(1): make comments optional

[Theo de Raadt]

> > That is why you are trying to push changes into an established
> > ecosystem, WITHOUT JUSTIFICATION.  Your only justification was
> > "because I want to do so".
> 
> Read this tread carefully and you'll see that I don't *push* anything. I
> don't consider one lonely patch on tech@ as pushing. tech@ is still the
> upstream for signify(1). Why not leave it alone if you don't like to
> merge it?
> 
> I'm not going to answer to another portion of pointless insults below.
> Just wondering how can you produce such amount of it out nothing.

Ivan, you bothered to write the diff and reply quite a few times, but
you waived all the concerns aside.

It's your problem not mine.

Re: signify(1): make comments optional

[Ivan Markin]

Theo de Raadt:
> That is why you are trying to push changes into an established
> ecosystem, WITHOUT JUSTIFICATION.  Your only justification was
> "because I want to do so".

Read this tread carefully and you'll see that I don't *push* anything. I
don't consider one lonely patch on tech@ as pushing. tech@ is still the
upstream for signify(1). Why not leave it alone if you don't like to
merge it?

I'm not going to answer to another portion of pointless insults below.
Just wondering how can you produce such amount of it out nothing.


> Look Ivan, don't be a child.  Justify your actions and your words.
> Explain what you are changing, and why.  Measure the impact.  Don't
> wave it away.  Waving away other people's concerns away does not count
> as jusification.  It makes you look like an uncaring ass.
> 
> If I was kind I would not even reply, because you are being a jerk
> only thinking of yourself.
> 
> If you cared so much about what signify does and why it is what it
> is, you would have invented it first.  But you didn't, did you.

--
Ivan Markin

Re: signify(1): make comments optional

[Theo de Raadt]

> Theo de Raadt:
> > You'll break other people's compatility without a thought.
> >
> > You only care about yourself.
> >
> > Yeah, that much is clear.
> 
> Theo, your insults are pointless. But your concerns are not.
> I don't want to break anything. I think that if something is useful
> enough for others one can find a way out without breaking anything.

But you do want to break compatibility.  Your own signify code
will soon generate files that the OpenBSD one cannot parse.

That is why you are trying to push changes into an established
ecosystem, WITHOUT JUSTIFICATION.  Your only justification was
"because I want to do so".

Look Ivan, don't be a child.  Justify your actions and your words.
Explain what you are changing, and why.  Measure the impact.  Don't
wave it away.  Waving away other people's concerns away does not count
as jusification.  It makes you look like an uncaring ass.

If I was kind I would not even reply, because you are being a jerk
only thinking of yourself.

If you cared so much about what signify does and why it is what it
is, you would have invented it first.  But you didn't, did you.

Re: signify(1): make comments optional

[Ivan Markin]

Theo de Raadt:
> You'll break other people's compatility without a thought.
>
> You only care about yourself.
>
> Yeah, that much is clear.

Theo, your insults are pointless. But your concerns are not.
I don't want to break anything. I think that if something is useful
enough for others one can find a way out without breaking anything.

--
Ivan Markin

Re: let head(1) understand `-' as stdin

[Jan Stary]

On Oct 12 23:23:18, t...@math.ethz.ch wrote:
> > Let me clarify the idea. 
> > If a filter recognizes '-' as a name for stdin,
> > then stdin can be one of the _multiple_ files being processed.
> > Filters that do not recognize '-' as a name, on the other hand,
> > only process stdin if it is the _only_ input.
> 
> I understand that - is convenient, but it's not strictly needed.
> If you need the standard input as one of several files (or as an
> explicit file argument), you can pass /dev/stdin.

This is probably what I was missing.
Thank you. Sorry for the noise.

Re: let head(1) understand `-' as stdin

[Theo Buehler]

> Let me clarify the idea. 
> If a filter recognizes '-' as a name for stdin,
> then stdin can be one of the _multiple_ files being processed.
> Filters that do not recognize '-' as a name, on the other hand,
> only process stdin if it is the _only_ input.

I understand that - is convenient, but it's not strictly needed.
If you need the standard input as one of several files (or as an
explicit file argument), you can pass /dev/stdin.

Re: let head(1) understand `-' as stdin

[Theo de Raadt]

>> > > The diff below makes head(1) recognize `-'
>> > > as a name for the standard input,
>> > > as many other utilities do.
>
>On Oct 11 23:55:26, schwa...@usta.de wrote:
>> > Do standards permit that extension?
>> 
>> POSIX neither requires nor forbids it, but encourages consistency
>> among all the utilities taking [file ...] arguments within a given
>> operating system:
>> 
>>   http://pubs.opengroup.org/onlinepubs/9699919799/utilities/head.html
>
>> > This is command used in scripts.  Scripts are often portable.  If one
>> > operating system has an extension, but others don't, then those
>> > scripts become unportable to use use of these extensions.
>
>[Ingo's detailed analysis snipped]
>
>> > I'm not raising a new argument here, it's been raised numerous times
>> > when it comes to commands commonly used in scripts.
>> > 
>> > So consider that first.
>> 
>> head(1) is firmly a BSD thingy, and all BSDs agree that "-" is a
>> file name and not standard input.  POSIX explicitly encourages
>> treating it as standard input ***if you do that for other utilities,
>> too***, and GNU coreutils has the only head(1) implementation i
>> found so far that actually does it.
>> 
>> The bigger picture seems to be that OpenBSD and illumos tend to resist
>> treating "-" as standard input whereever resisting is allowed,
>> while GNU embraces treating "-" as standard whereever allowed.
>> Most other systems seem to be somewhat inconsistent, in particular
>> in those cases where they imported GNU utilities.
>> 
>> So much for the facts.
>> 
>> 
>> I see two ways forward that make sense to me:
>> 
>>  a) Either remain conservative - in line with both BSD and SysV
>> heritage - and not do it unless required by the standard.
>> 
>>  b) Or switch over to doing it whereever allowed - but then we
>> should do it not just for head(1), but also for tail(1),
>> grep(1), sed(1) and probably several others, and then we
>> should probably also try to push such patches to FreeBSD,
>> DragonFly, NetBSD, and illumos, or at least give them heads-ups.
>> 
>> Changing only head(1) and leaving everything else as it is does not
>> look like a complete plan to me.  Even POSIX wouldn't encourage that.
>
>Thank you for the detailed analysis.
>
>If there is any interest in possibly going b)
>see below for a look at the other text filters.
>
>
>Let me clarify the idea. 
>If a filter recognizes '-' as a name for stdin,
>then stdin can be one of the _multiple_ files being processed.
>Filters that do not recognize '-' as a name, on the other hand,
>only process stdin if it is the _only_ input.
>
>For example cat(1) and paste(1) do that, head(1) and tail(1) don't.
>And there are other utilities that could do that, but don't.
>Below is a list of text filters from bin/ and usr.bin/
>for which this seems relevant, separated into the two camps.
>
>   Jan
>
>
>These recognize '-' as a name for stdin
>as one of possibly many inputs:
>
>   cat
>   cmp
>   comm
>   cut
>   diff
>   file
>   join
>   lam
>   paste
>   pr
>   sdiff
>   sort
>
>
>These process stdin only if it is
>the only (unnamed) input:
>
>   column  
>   expand
>   fmt
>   fold
>   grep
>   head
>   hexdump
>   nl
>   rev
>   tail
>   ul
>   unexpand
>   unvis
>   vis
>   wc

Bobby has pulled on Sally's hair, to everyone can pull on everyone's
hair.  Your list means nothing.  Please read the standards fully, and
understand the legal lawyer thing.

Re: let head(1) understand `-' as stdin

[Jan Stary]

> > > The diff below makes head(1) recognize `-'
> > > as a name for the standard input,
> > > as many other utilities do.

On Oct 11 23:55:26, schwa...@usta.de wrote:
> > Do standards permit that extension?
> 
> POSIX neither requires nor forbids it, but encourages consistency
> among all the utilities taking [file ...] arguments within a given
> operating system:
> 
>   http://pubs.opengroup.org/onlinepubs/9699919799/utilities/head.html

> > This is command used in scripts.  Scripts are often portable.  If one
> > operating system has an extension, but others don't, then those
> > scripts become unportable to use use of these extensions.

[Ingo's detailed analysis snipped]

> > I'm not raising a new argument here, it's been raised numerous times
> > when it comes to commands commonly used in scripts.
> > 
> > So consider that first.
> 
> head(1) is firmly a BSD thingy, and all BSDs agree that "-" is a
> file name and not standard input.  POSIX explicitly encourages
> treating it as standard input ***if you do that for other utilities,
> too***, and GNU coreutils has the only head(1) implementation i
> found so far that actually does it.
> 
> The bigger picture seems to be that OpenBSD and illumos tend to resist
> treating "-" as standard input whereever resisting is allowed,
> while GNU embraces treating "-" as standard whereever allowed.
> Most other systems seem to be somewhat inconsistent, in particular
> in those cases where they imported GNU utilities.
> 
> So much for the facts.
> 
> 
> I see two ways forward that make sense to me:
> 
>  a) Either remain conservative - in line with both BSD and SysV
> heritage - and not do it unless required by the standard.
> 
>  b) Or switch over to doing it whereever allowed - but then we
> should do it not just for head(1), but also for tail(1),
> grep(1), sed(1) and probably several others, and then we
> should probably also try to push such patches to FreeBSD,
> DragonFly, NetBSD, and illumos, or at least give them heads-ups.
> 
> Changing only head(1) and leaving everything else as it is does not
> look like a complete plan to me.  Even POSIX wouldn't encourage that.

Thank you for the detailed analysis.

If there is any interest in possibly going b)
see below for a look at the other text filters.


Let me clarify the idea. 
If a filter recognizes '-' as a name for stdin,
then stdin can be one of the _multiple_ files being processed.
Filters that do not recognize '-' as a name, on the other hand,
only process stdin if it is the _only_ input.

For example cat(1) and paste(1) do that, head(1) and tail(1) don't.
And there are other utilities that could do that, but don't.
Below is a list of text filters from bin/ and usr.bin/
for which this seems relevant, separated into the two camps.

Jan


These recognize '-' as a name for stdin
as one of possibly many inputs:

cat
cmp
comm
cut
diff
file
join
lam
paste
pr
sdiff
sort


These process stdin only if it is
the only (unnamed) input:

column  
expand
fmt
fold
grep
head
hexdump
nl
rev
tail
ul
unexpand
unvis
vis
wc

Re: systat(1) hostname

[Alexander Bluhm]

On Wed, Oct 12, 2016 at 03:20:00PM +0200, Otto Moerbeek wrote:
> simple diff to show the hostname on the second line. OK?

OK bluhm@

> 
> BTW, batch mode doesn't function here as expected. Need to look into that,
> 
>   -Otto
> 
> Index: main.c
> ===
> RCS file: /cvs/src/usr.bin/systat/main.c,v
> retrieving revision 1.64
> diff -u -p -r1.64 main.c
> --- main.c2 Jan 2016 15:02:05 -   1.64
> +++ main.c8 Oct 2016 13:34:07 -
> @@ -124,8 +124,10 @@ print_header(void)
>  
>   if (rawmode)
>   printf("\n\n%s\n", header);
> - else
> + else {
>   mvprintw(0, 0, "%s", header);
> + mvprintw(1, 1, "%s", hostname);
> + }
>  
>   return (1);
>  }

/usr/src beforeinstall: make prereq as BUILDUSER

[Theo Buehler]

Several people noticed that a few files from libstdc++-v3 in /usr/obj
end up being owned by root, namely:

c++config.h gthr.h gthr-single.h gthr-posix.h gthr-tpf.h gthr-default.h

The problem is that they are regenerated by root when beforeinstall does
'make includes' directly from /usr/src/includes.  A cheap fix is to call
make includes from the main Makefile, which was already fixed to
de-escalate the prereq step.

Fixing that directly in gnu/lib/libstdc++-v3/Makefile would also be
possible, but it seems to be quite a bit messier since one would have to
touch quite a few targets to do the proper de-escalation.

With that, /usr/obj contains no root-owned files after 'make build'.

Index: Makefile
===
RCS file: /var/cvs/src/Makefile,v
retrieving revision 1.129
diff -u -p -r1.129 Makefile
--- Makefile6 Oct 2016 18:56:17 -   1.129
+++ Makefile12 Oct 2016 20:14:58 -
@@ -57,7 +57,7 @@ includes:
 beforeinstall:
cd ${.CURDIR}/etc && exec ${MAKE} DESTDIR=${DESTDIR} distrib-dirs
cd ${.CURDIR}/etc && exec ${MAKE} DESTDIR=${DESTDIR} install-mtree
-   cd ${.CURDIR}/include && exec ${MAKE} includes
+   exec ${MAKE} includes
 
 afterinstall:
 .ifndef NOMAN

Re: vmd/vmctl load/reload/reset

[Mike Larkin]

On Wed, Oct 12, 2016 at 02:06:35PM +0200, Reyk Floeter wrote:
> On Wed, Oct 12, 2016 at 01:44:25PM +0200, Reyk Floeter wrote:
> > Hi,
> > 
> > vmctl reload is currently broken, the attached diff fixes it and
> > re-introduces the semantics that originally came from iked:
> > 
> > - load/reload just reloads the configuration without clearing any
> > running configuration.  This way you can start vmd with a few
> > configured vms, terminate one vm, and reload the configuration which
> > will restart the terminated vm but keep the running ones.
> > 
> > - reset clears the configuration (and possibly terminates vms) without
> > reloading the configuration.  "vmctl reset" thus terminates all vms.
> > 
> > # vmctl load /etc/my-personal-vm.conf
> > # vmctl reload
> > # vmctl reset
> > 
> > OK?
> > 
> 
> ajacoutot@ reminded me of SIGHUP:
> change it to reload instead of reset on HUP.
> 
> Updated diff, OK?
> 
> Reyk
> 

sure

> Index: usr.sbin/vmctl/main.c
> ===
> RCS file: /cvs/src/usr.sbin/vmctl/main.c,v
> retrieving revision 1.17
> diff -u -p -u -p -r1.17 main.c
> --- usr.sbin/vmctl/main.c 10 May 2016 11:00:54 -  1.17
> +++ usr.sbin/vmctl/main.c 12 Oct 2016 12:01:14 -
> @@ -50,6 +50,8 @@ int  vmm_action(struct parse_result *);
>  int   ctl_console(struct parse_result *, int, char *[]);
>  int   ctl_create(struct parse_result *, int, char *[]);
>  int   ctl_load(struct parse_result *, int, char *[]);
> +int   ctl_reload(struct parse_result *, int, char *[]);
> +int   ctl_reset(struct parse_result *, int, char *[]);
>  int   ctl_start(struct parse_result *, int, char *[]);
>  int   ctl_status(struct parse_result *, int, char *[]);
>  int   ctl_stop(struct parse_result *, int, char *[]);
> @@ -57,8 +59,9 @@ int  ctl_stop(struct parse_result *, in
>  struct ctl_command ctl_commands[] = {
>   { "console",CMD_CONSOLE,ctl_console,"id" },
>   { "create", CMD_CREATE, ctl_create, "\"path\" -s size", 1 },
> - { "load",   CMD_LOAD,   ctl_load,   "[path]" },
> - { "reload", CMD_RELOAD, ctl_load,   "[path]" },
> + { "load",   CMD_LOAD,   ctl_load,   "\"path\"" },
> + { "reload", CMD_RELOAD, ctl_reload, "" },
> + { "reset",  CMD_RESET,  ctl_reset,  "[all|vms|switches]" },
>   { "start",  CMD_START,  ctl_start,  "\"name\""
>   " [-c] -k kernel -m size [-i count] [-d disk]*" },
>   { "status", CMD_STATUS, ctl_status, "[id]" },
> @@ -204,14 +207,18 @@ vmmaction(struct parse_result *res)
>   case CMD_CONSOLE:
>   get_info_vm(res->id, res->name, 1);
>   break;
> + case CMD_LOAD:
> + imsg_compose(ibuf, IMSG_VMDOP_LOAD, 0, 0, -1,
> + res->path, strlen(res->path) + 1);
> + done = 1;
> + break;
>   case CMD_RELOAD:
> - imsg_compose(ibuf, IMSG_VMDOP_RELOAD, 0, 0, -1,
> - res->path, res->path == NULL ? 0 : strlen(res->path) + 1);
> + imsg_compose(ibuf, IMSG_VMDOP_RELOAD, 0, 0, -1, NULL, 0);
>   done = 1;
>   break;
> - case CMD_LOAD:
> - imsg_compose(ibuf, IMSG_VMDOP_LOAD, 0, 0, -1,
> - res->path, res->path == NULL ? 0 : strlen(res->path) + 1);
> + case CMD_RESET:
> + imsg_compose(ibuf, IMSG_CTL_RESET, 0, 0, -1,
> + >mode, sizeof(res->mode));
>   done = 1;
>   break;
>   case CMD_CREATE:
> @@ -431,16 +438,41 @@ ctl_status(struct parse_result *res, int
>  int
>  ctl_load(struct parse_result *res, int argc, char *argv[])
>  {
> - char*config_file = NULL;
> -
> - if (argc == 2)
> - config_file = argv[1];
> - else if (argc > 2)
> + if (argc != 2)
>   ctl_usage(res->ctl);
>  
> - if (config_file != NULL &&
> - (res->path = strdup(config_file)) == NULL)
> + if ((res->path = strdup(argv[1])) == NULL)
>   err(1, "strdup");
> +
> + return (vmmaction(res));
> +}
> +
> +int
> +ctl_reload(struct parse_result *res, int argc, char *argv[])
> +{
> + if (argc != 1)
> + ctl_usage(res->ctl);
> +
> + return (vmmaction(res));
> +}
> +
> +int
> +ctl_reset(struct parse_result *res, int argc, char *argv[])
> +{
> + if (argc == 2) {
> + if (strcasecmp("all", argv[1]) == 0)
> + res->mode = CONFIG_ALL;
> + else if (strcasecmp("vms", argv[1]) == 0)
> + res->mode = CONFIG_VMS;
> + else if (strcasecmp("switches", argv[1]) == 0)
> + res->mode = CONFIG_SWITCHES;
> + else
> + ctl_usage(res->ctl);
> + } else if (argc > 2)
> + ctl_usage(res->ctl);
> +
> + if (res->mode == 0)
> 

Re: logname turd polish

[Ingo Schwarze]

Hi,

Todd C. Miller wrote on Wed, Oct 12, 2016 at 09:57:07AM -0600:
> On Wed, 12 Oct 2016 15:03:04 +0200, Ingo Schwarze wrote:
 
>> please just commit this patch you sent out back in July.
>> The old, verbose style keeps confusing people,
>> see for example Jan Stary's recent messages to tech@.
>> I guess you just overlooked my OK.
>> There was no opposition when you put this on tech@.

> Note that this changes the behavior slightly when an option is
> specified:
> 
> Before:
> $ logname -z
> logname: unknown option -- z
> usage: logname
> 
> After:
> $ logname -z
> usage: logname
> 
> Depending on your point of view, this could be considered a feature.
> Personally, I prefer the getopt(3) way.

Oops, i missed that because i already had tedu's version in /usr/bin/
for several months.

So, here is a version of tedu@'s cleanup containing only the parts
that seem uncontroversial, even catering to joerg@'s remark that
somebody might add an option at some point - though i do hope it
doesn't come to that, 'cause id(1) already exists.

OK?
  Ingo


Index: logname.c
===
RCS file: /cvs/src/usr.bin/logname/logname.c,v
retrieving revision 1.9
diff -u -p -r1.9 logname.c
--- logname.c   9 Oct 2015 01:37:08 -   1.9
+++ logname.c   12 Oct 2016 17:48:26 -
@@ -30,13 +30,17 @@
  * SUCH DAMAGE.
  */
 
+#include 
 #include 
 #include 
-#include 
 #include 
-#include 
 
-void usage(void);
+static void __dead
+usage(void)
+{
+   (void)fprintf(stderr, "usage: logname\n");
+   exit(1);
+}
 
 int
 main(int argc, char *argv[])
@@ -44,33 +48,21 @@ main(int argc, char *argv[])
int ch;
char *p;
 
-   setlocale(LC_ALL, "");
-
if (pledge("stdio", NULL) == -1)
err(1, "pledge");
 
while ((ch = getopt(argc, argv, "")) != -1)
switch (ch) {
-   case '?':
default:
usage();
-   /* NOTREACHED */
}
 
-   if (argc != optind) {
+   if (argc != optind)
usage();
-   /* NOTREACHED */
-   }
 
if ((p = getlogin()) == NULL)
err(1, NULL);
-   (void)printf("%s\n", p);
-   exit(0);
-}
 
-void
-usage(void)
-{
-   (void)fprintf(stderr, "usage: logname\n");
-   exit(1);
+   (void)printf("%s\n", p);
+   return 0;
 }

Re: crontab(5): document -q flag in command

[Ingo Schwarze]

Hi Todd,

Todd C. Miller wrote on Wed, Oct 12, 2016 at 10:35:05AM -0600:
> On Wed, 12 Oct 2016 18:32:42 +0200, Ingo Schwarze wrote:

>> So, OK to commit my version?

> OK,

Committed, thanks for checking.

> though that seems like a bug in the crontab parser.

NetBSD has the same code and behaviour.
FreeBSD, DragonFly, illumos don't support -q at all.
Linux doesn't support it (testing only, no code read).
Solaris 11 doesn't support it (testing only, no code read).

Probably, it was a bad idea to implement that feature,
but now may be the wrong time to delete it.

In any case, we seem free to change it.
I'm not opposed to it if somebody considers it a bug and wants to fix it.

But i recommend leaving the stricter language in the manual
because versions exist in the wild requiring the whitespace,
so putting some whitespace there seems like best practice.

Yours,
  Ingo

Re: tcpdump: decode Large BGP Communities

[Stuart Henderson]

On 2016/10/12 18:44, Job Snijders wrote:
> This patch adds support to tcpdump(8) to decode Large BGP
> Communities in human readable form.
> 
> Example:
> 
>   [ snip ] BGP (UPDATE: (Path attributes: (ORIGIN[T] IGP)
>   (AS_PATH[T] 65000)
>   (NEXT_HOP[T] pxtr-2.meerval.net)
>   (COMMUNITIES[OT] 666:666 2914:0)
>   (LARGE_COMMUNITIES[OT] 2914:0:666 2914:4294927296:123))
>   (NLRI: 9.9.9.10/32)) (DF) (ttl 63, id 2806, len 138)
> 
> Kind regards,
> 
> Job

OK sthen@.


> 
> Index: src/usr.sbin/tcpdump/print-bgp.c
> ===
> RCS file: /cvs/src/usr.sbin/tcpdump/print-bgp.c,v
> retrieving revision 1.18
> diff -u -p -u -r1.18 print-bgp.c
> --- src/usr.sbin/tcpdump/print-bgp.c  20 Oct 2015 11:29:07 -  1.18
> +++ src/usr.sbin/tcpdump/print-bgp.c  12 Oct 2016 16:09:51 -
> @@ -134,6 +134,7 @@ struct bgp_attr {
>  #define BGPTYPE_EXTD_COMMUNITIES 16  /* RFC4360 */
>  #define BGPTYPE_AS4_PATH 17  /* RFC4893 */
>  #define BGPTYPE_AGGREGATOR4  18  /* RFC4893 */
> +#define BGPTYPE_LARGE_COMMUNITIES30  /* 
> draft-ietf-idr-large-community */
>  
>  #define BGP_AS_SET 1
>  #define BGP_AS_SEQUENCE2
> @@ -265,7 +266,8 @@ static const char *bgpattr_type[] = {
>   "MULTI_EXIT_DISC", "LOCAL_PREF", "ATOMIC_AGGREGATE", "AGGREGATOR",
>   "COMMUNITIES", "ORIGINATOR_ID", "CLUSTER_LIST", "DPA",
>   "ADVERTISERS", "RCID_PATH", "MP_REACH_NLRI", "MP_UNREACH_NLRI",
> - "EXTD_COMMUNITIES", "AS4_PATH", "AGGREGATOR4",
> + "EXTD_COMMUNITIES", "AS4_PATH", "AGGREGATOR4", NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, "LARGE_COMMUNITIES",
>  };
>  #define bgp_attr_type(x) \
>   num_or_str(bgpattr_type, \
> @@ -544,6 +546,21 @@ bgp_attr_print(const struct bgp_attr *at
>   }
>   tlen -= 4;
>   p += 4;
> + }
> + break;
> + case BGPTYPE_LARGE_COMMUNITIES:
> + if (len == 0 || len % 12) {
> + printf(" invalid len");
> + break;
> + }
> + while (tlen>0) {
> + TCHECK2(p[0], 12);
> + printf(" %u:%u:%u",
> + EXTRACT_32BITS(p),
> + EXTRACT_32BITS(p + 4),
> + EXTRACT_32BITS(p + 8));
> + tlen -= 12;
> + p += 12;
>   }
>   break;
>   case BGPTYPE_ORIGINATOR_ID:
> 

tcpdump: decode Large BGP Communities

[Job Snijders]

This patch adds support to tcpdump(8) to decode Large BGP
Communities in human readable form.

Example:

[ snip ] BGP (UPDATE: (Path attributes: (ORIGIN[T] IGP)
(AS_PATH[T] 65000)
(NEXT_HOP[T] pxtr-2.meerval.net)
(COMMUNITIES[OT] 666:666 2914:0)
(LARGE_COMMUNITIES[OT] 2914:0:666 2914:4294927296:123))
(NLRI: 9.9.9.10/32)) (DF) (ttl 63, id 2806, len 138)

Kind regards,

Job


Index: src/usr.sbin/tcpdump/print-bgp.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/print-bgp.c,v
retrieving revision 1.18
diff -u -p -u -r1.18 print-bgp.c
--- src/usr.sbin/tcpdump/print-bgp.c20 Oct 2015 11:29:07 -  1.18
+++ src/usr.sbin/tcpdump/print-bgp.c12 Oct 2016 16:09:51 -
@@ -134,6 +134,7 @@ struct bgp_attr {
 #define BGPTYPE_EXTD_COMMUNITIES   16  /* RFC4360 */
 #define BGPTYPE_AS4_PATH   17  /* RFC4893 */
 #define BGPTYPE_AGGREGATOR418  /* RFC4893 */
+#define BGPTYPE_LARGE_COMMUNITIES  30  /* 
draft-ietf-idr-large-community */
 
 #define BGP_AS_SET 1
 #define BGP_AS_SEQUENCE2
@@ -265,7 +266,8 @@ static const char *bgpattr_type[] = {
"MULTI_EXIT_DISC", "LOCAL_PREF", "ATOMIC_AGGREGATE", "AGGREGATOR",
"COMMUNITIES", "ORIGINATOR_ID", "CLUSTER_LIST", "DPA",
"ADVERTISERS", "RCID_PATH", "MP_REACH_NLRI", "MP_UNREACH_NLRI",
-   "EXTD_COMMUNITIES", "AS4_PATH", "AGGREGATOR4",
+   "EXTD_COMMUNITIES", "AS4_PATH", "AGGREGATOR4", NULL, NULL, NULL,
+   NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, "LARGE_COMMUNITIES",
 };
 #define bgp_attr_type(x) \
num_or_str(bgpattr_type, \
@@ -544,6 +546,21 @@ bgp_attr_print(const struct bgp_attr *at
}
tlen -= 4;
p += 4;
+   }
+   break;
+   case BGPTYPE_LARGE_COMMUNITIES:
+   if (len == 0 || len % 12) {
+   printf(" invalid len");
+   break;
+   }
+   while (tlen>0) {
+   TCHECK2(p[0], 12);
+   printf(" %u:%u:%u",
+   EXTRACT_32BITS(p),
+   EXTRACT_32BITS(p + 4),
+   EXTRACT_32BITS(p + 8));
+   tlen -= 12;
+   p += 12;
}
break;
case BGPTYPE_ORIGINATOR_ID:

Re: crontab(5): document -q flag in command

[Ingo Schwarze]

Hi Todd,

Todd C. Miller wrote on Wed, Oct 12, 2016 at 10:06:07AM -0600:

> Whitespace between the -q and the command is optional, not required.

No, according to my reading of the code and my testing, it is required.

The code say:

case 'q':
e->flags |= DONT_LOG;
Skip_Nonblanks(ch, file)
break;

So, if you don't put whitespace, the next word gets skipped,
and indeed:

  schwarze@isnote $ cat /etc/crontab  
  18 * * * * schwarze -qecho echo text > /tmp/cron_out_file
  21 * * * * schwarze echo echo text > /tmp/cron_out_file_noq

  schwarze@isnote $ ll /tmp/cron*
  -rw-r--r--  1 schwarze  wheel   5 Oct 12 18:18 /tmp/cron_out_file
  -rw-r--r--  1 schwarze  wheel  10 Oct 12 18:21 /tmp/cron_out_file_noq

  schwarze@isnote $ head /tmp/cron*
  ==> /tmp/cron_out_file <==
  text

  ==> /tmp/cron_out_file_noq <==
  echo text

So, OK to commit my version?

Yours,
  Ingo


Index: crontab.5
===
RCS file: /cvs/src/usr.sbin/cron/crontab.5,v
retrieving revision 1.33
diff -u -p -r1.33 crontab.5
--- crontab.5   30 Jan 2014 20:02:42 -  1.33
+++ crontab.5   12 Oct 2016 16:28:59 -
@@ -193,6 +193,14 @@ will be changed into newline characters,
 after the first
 .Ql %
 will be sent to the command as standard input.
+If the
+.Ar command
+field begins with
+.Ql -q ,
+execution will not be logged.
+Use whitespace to separate
+.Ql -q
+from the command.
 .Pp
 Commands are executed by
 .Xr cron 8
@@ -320,6 +328,9 @@ Ranges may include
 .Dq steps .
 .It
 Months or days of the week can be specified by name.
+.It
+Logging can be suppressed with
+.Ql -q .
 .It
 Environment variables can be set in a crontab.
 .It


> Index: crontab.5
> ===
> RCS file: /cvs/src/usr.sbin/cron/crontab.5,v
> retrieving revision 1.33
> diff -u -p -u -r1.33 crontab.5
> --- crontab.5 30 Jan 2014 20:02:42 -  1.33
> +++ crontab.5 12 Oct 2016 16:05:53 -
> @@ -193,6 +193,14 @@ will be changed into newline characters,
>  after the first
>  .Ql %
>  will be sent to the command as standard input.
> +If the
> +.Ar command
> +field begins with
> +.Ql -q ,
> +execution will not be logged.
> +Any whitespace between the
> +.Ql -q
> +and the command that follows it is ignored.
>  .Pp
>  Commands are executed by
>  .Xr cron 8

Re: crontab(5): document -q flag in command

[Todd C. Miller]

Whitespace between the -q and the command is optional, not required.

 - todd

Index: crontab.5
===
RCS file: /cvs/src/usr.sbin/cron/crontab.5,v
retrieving revision 1.33
diff -u -p -u -r1.33 crontab.5
--- crontab.5   30 Jan 2014 20:02:42 -  1.33
+++ crontab.5   12 Oct 2016 16:05:53 -
@@ -193,6 +193,14 @@ will be changed into newline characters,
 after the first
 .Ql %
 will be sent to the command as standard input.
+If the
+.Ar command
+field begins with
+.Ql -q ,
+execution will not be logged.
+Any whitespace between the
+.Ql -q
+and the command that follows it is ignored.
 .Pp
 Commands are executed by
 .Xr cron 8

Re: PING: Re: logname turd polish

[Todd C. Miller]

On Wed, 12 Oct 2016 15:03:04 +0200, Ingo Schwarze wrote:

> please just commit this patch you sent out back in July.
> The old, verbose style keeps confusing people,
> see for example Jan Stary's recent messages to tech@.
> I guess you just overlooked my OK.
> There was no opposition when you put this on tech@.

Note that this changes the behavior slightly when an option is
specified:

Before:
$ logname -z
logname: unknown option -- z
usage: logname

After:
$ logname -z
usage: logname

Depending on your point of view, this could be considered a feature.
Personally, I prefer the getopt(3) way.

 - todd

switchd(8): add flow_mod validation

[Rafael Zalamena]

This diff teaches switchd(8) how to validate flow_mod messages, more
specifically the flow instructions and actions. The oxm validations
were already implemented so we get them for free here.

ok?

Index: sys/net/ofp.h
===
RCS file: /cvs/src/sys/net/ofp.h,v
retrieving revision 1.2
diff -u -p -r1.2 ofp.h
--- sys/net/ofp.h   30 Sep 2016 12:40:00 -  1.2
+++ sys/net/ofp.h   12 Oct 2016 15:36:30 -
@@ -315,14 +315,14 @@ struct ofp_action_push {
uint16_tap_type;
uint16_tap_len;
uint16_tap_ethertype;
-   uint8_t pad[2];
+   uint8_t ap_pad[2];
 } __packed;
 
 struct ofp_action_pop_mpls {
uint16_tapm_type;
uint16_tapm_len;
uint16_tapm_ethertype;
-   uint8_t pad[2];
+   uint8_t apm_pad[2];
 } __packed;
 
 struct ofp_action_group {
@@ -342,6 +342,12 @@ struct ofp_action_set_field {
uint16_tasf_type;
uint16_tasf_len;
uint8_t asf_field[4];
+} __packed;
+
+struct ofp_action_set_queue {
+   uint16_tasq_type;
+   uint16_tasq_len;
+   uint32_tasq_queue_id;
 } __packed;
 
 /* Packet-Out Message */
Index: usr.sbin/switchd/ofp13.c
===
RCS file: /cvs/src/usr.sbin/switchd/ofp13.c,v
retrieving revision 1.20
diff -u -p -r1.20 ofp13.c
--- usr.sbin/switchd/ofp13.c12 Oct 2016 15:18:56 -  1.20
+++ usr.sbin/switchd/ofp13.c12 Oct 2016 15:36:30 -
@@ -54,6 +54,12 @@ int   ofp13_echo_request(struct switchd *
 int ofp13_validate_error(struct switchd *,
struct sockaddr_storage *, struct sockaddr_storage *,
struct ofp_header *, struct ibuf *);
+int ofp13_validate_action(struct switchd *, struct ofp_header *,
+   struct ibuf *, off_t *, struct ofp_action_header *);
+int ofp13_validate_instruction(struct switchd *, struct ofp_header *,
+   struct ibuf *, off_t *, struct ofp_instruction *);
+int ofp13_validate_flow_mod(struct switchd *, struct sockaddr_storage *,
+   struct sockaddr_storage *, struct ofp_header *, struct ibuf *);
 int ofp13_validate_oxm_basic(struct ibuf *, off_t, int, uint8_t);
 int ofp13_validate_oxm(struct switchd *, struct ofp_ox_match *,
struct ofp_header *, struct ibuf *, off_t);
@@ -121,7 +127,7 @@ struct ofp_callback ofp13_callbacks[] = 
{ OFP_T_FLOW_REMOVED,   ofp13_flow_removed, NULL },
{ OFP_T_PORT_STATUS,NULL, NULL },
{ OFP_T_PACKET_OUT, NULL, ofp13_validate_packet_out },
-   { OFP_T_FLOW_MOD,   NULL, NULL },
+   { OFP_T_FLOW_MOD,   NULL, ofp13_validate_flow_mod },
{ OFP_T_GROUP_MOD,  NULL, NULL },
{ OFP_T_PORT_MOD,   NULL, NULL },
{ OFP_T_TABLE_MOD,  NULL, NULL },
@@ -501,6 +507,274 @@ ofp13_validate_packet_out(struct switchd
if (pout->pout_buffer_id == (uint32_t)-1)
break;
off += ntohs(ah->ah_len);
+   }
+
+   return (0);
+}
+
+int
+ofp13_validate_action(struct switchd *sc, struct ofp_header *oh,
+struct ibuf *ibuf, off_t *off, struct ofp_action_header *ah)
+{
+   struct ofp_action_output*ao;
+   struct ofp_action_mpls_ttl  *amt;
+   struct ofp_action_push  *ap;
+   struct ofp_action_pop_mpls  *apm;
+   struct ofp_action_group *ag;
+   struct ofp_action_nw_ttl*ant;
+   struct ofp_action_set_field *asf;
+   struct ofp_action_set_queue *asq;
+   struct ofp_ox_match *oxm;
+   int  len, type;
+   off_tmoff;
+
+   type = ntohs(ah->ah_type);
+   len = ntohs(ah->ah_len);
+   switch (type) {
+   case OFP_ACTION_OUTPUT:
+   if ((ao = ibuf_seek(ibuf, *off, sizeof(*ao))) == NULL)
+   return (-1);
+
+   *off += len;
+   log_debug("\t\taction %s len %d port %u max_len %d",
+   print_map(type, ofp_action_map), len, ntohl(ao->ao_port),
+   ntohs(ao->ao_max_len));
+   break;
+   case OFP_ACTION_SET_MPLS_TTL:
+   if ((amt = ibuf_seek(ibuf, *off, sizeof(*amt))) == NULL)
+   return (-1);
+
+   *off += len;
+   log_debug("\t\taction %s len %d ttl %d",
+   print_map(type, ofp_action_map), len, amt->amt_ttl);
+   break;
+   case OFP_ACTION_PUSH_VLAN:
+   case OFP_ACTION_PUSH_MPLS:
+   case OFP_ACTION_PUSH_PBB:
+   if ((ap = ibuf_seek(ibuf, *off, sizeof(*ap))) == NULL)
+   return (-1);
+
+   *off += len;
+   log_debug("\t\taction 

Re: crontab(5): document -q flag in command

[Jérémie Courrèges-Anglas]

Ingo Schwarze  writes:

> Hi Jeremie,
>
> Jeremie Courreges-Anglas wrote on Wed, Oct 12, 2016 at 03:19:13PM +0200:
>> Wouter Clarie  writes:
>
>>> The -q flag for the command in a crontab(5) entry was introduced
>>> in revision 1.8 of src/usr.sbin/cron/entry.c back in 2001,
>>> but never documented.
>
>> Good catch.
>
>>> (Sorry, I'm no hero with mdoc, so not sure if the markup is correct.)
>
>> I'm no mdoc here either, but:
>> - I'd drop the first Pp, IMO -q should be described in the same
>>   paragraph as other special characters.
>
> That sounds reasonable.
>
>> - I find it weird to use Fl to describe what is technically not
>>   a command-line option.  I went with Ql, just like for % and \.
>
> I agree, .Ql is good here.
>
>> Updated diff, assuming the point above are valid; input welcome.
>
> That's already an improvement, so OK schwarze@ if you want to commit.
>
> However, more is missing.
>
>  - It is non-obvious how to delimit "-q".
>If i read the code correctly, it first discards any non-blank
>characters that follow "-q" up to the first blank, and then it
>discards all blank characters up to the next non-blank,
>assuming that non-blank starts the command.
>That logic is too complicated for the manual to explain,
>so i think the manual ought to require a stricter syntax
>and leave behaviour unspecified if people don't stick to that.
>
>I think i would say something like:
>
>+If the
>+.Ar command
>+field starts with
>+.Ql -q ,
>+execution will not be logged.
>+Use whitespace to separate
>+.Ql -q
>+from the command.

Right, just commit this please, ok jca@

>  - STANDARDS should mention that -q is an extension.

/me rolls eyes, discovering that crontab(5) was standardized by POSIX...

> Yours,
>   Ingo
>
>
>> Index: crontab.5
>> ===
>> RCS file: /d/cvs/src/usr.sbin/cron/crontab.5,v
>> retrieving revision 1.33
>> diff -u -p -p -u -r1.33 crontab.5
>> --- crontab.530 Jan 2014 20:02:42 -  1.33
>> +++ crontab.512 Oct 2016 13:13:56 -
>> @@ -193,6 +193,11 @@ will be changed into newline characters,
>>  after the first
>>  .Ql %
>>  will be sent to the command as standard input.
>> +If the
>> +.Ar command
>> +field starts with
>> +.Ql -q ,
>> +execution will not be logged.
>>  .Pp
>>  Commands are executed by
>>  .Xr cron 8

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: enforce zero options

[Joerg Sonnenberger]

On Wed, Oct 12, 2016 at 03:53:17PM +0200, Jan Stary wrote:
> I don't get it: why do we need to handle --
> in utils which take no options and no arguments?

Are you sure they will never handle options in the future?

Joerg

Re: crontab(5): document -q flag in command

[Ingo Schwarze]

Hi Jeremie,

Jeremie Courreges-Anglas wrote on Wed, Oct 12, 2016 at 03:19:13PM +0200:
> Wouter Clarie  writes:

>> The -q flag for the command in a crontab(5) entry was introduced
>> in revision 1.8 of src/usr.sbin/cron/entry.c back in 2001,
>> but never documented.

> Good catch.

>> (Sorry, I'm no hero with mdoc, so not sure if the markup is correct.)

> I'm no mdoc here either, but:
> - I'd drop the first Pp, IMO -q should be described in the same
>   paragraph as other special characters.

That sounds reasonable.

> - I find it weird to use Fl to describe what is technically not
>   a command-line option.  I went with Ql, just like for % and \.

I agree, .Ql is good here.

> Updated diff, assuming the point above are valid; input welcome.

That's already an improvement, so OK schwarze@ if you want to commit.

However, more is missing.

 - It is non-obvious how to delimit "-q".
   If i read the code correctly, it first discards any non-blank
   characters that follow "-q" up to the first blank, and then it
   discards all blank characters up to the next non-blank,
   assuming that non-blank starts the command.
   That logic is too complicated for the manual to explain,
   so i think the manual ought to require a stricter syntax
   and leave behaviour unspecified if people don't stick to that.

   I think i would say something like:

   +If the
   +.Ar command
   +field starts with
   +.Ql -q ,
   +execution will not be logged.
   +Use whitespace to separate
   +.Ql -q
   +from the command.

 - STANDARDS should mention that -q is an extension.

Yours,
  Ingo


> Index: crontab.5
> ===
> RCS file: /d/cvs/src/usr.sbin/cron/crontab.5,v
> retrieving revision 1.33
> diff -u -p -p -u -r1.33 crontab.5
> --- crontab.5 30 Jan 2014 20:02:42 -  1.33
> +++ crontab.5 12 Oct 2016 13:13:56 -
> @@ -193,6 +193,11 @@ will be changed into newline characters,
>  after the first
>  .Ql %
>  will be sent to the command as standard input.
> +If the
> +.Ar command
> +field starts with
> +.Ql -q ,
> +execution will not be logged.
>  .Pp
>  Commands are executed by
>  .Xr cron 8

Fix bpf_catchpacket comment

[Jeremie Courreges-Anglas]

This function doesn't return whether listeners should be woken up, it
calls bpf_wakeup as needed.  ok?


Index: bpf.c
===
RCS file: /d/cvs/src/sys/net/bpf.c,v
retrieving revision 1.149
diff -u -p -p -u -r1.149 bpf.c
--- bpf.c   12 Sep 2016 16:24:37 -  1.149
+++ bpf.c   12 Oct 2016 13:52:39 -
@@ -1343,8 +1343,8 @@ bpf_mtap_ether(caddr_t arg, const struct
 
 /*
  * Move the packet data from interface memory (pkt) into the
- * store buffer.  Return 1 if it's time to wakeup a listener (buffer full),
- * otherwise 0.  "copy" is the routine called to do the actual data
+ * store buffer.  Wake up listeners if needed.
+ * "copy" is the routine called to do the actual data
  * transfer.  bcopy is passed in to copy contiguous chunks, while
  * bpf_mcopy is passed in to copy mbuf chains.  In the latter case,
  * pkt is really an mbuf.


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: enforce zero options

[Jan Stary]

On Oct 12 15:00:23, j...@wxcvbn.org wrote:
> Jan Stary  writes:
> 
> > Some programs in bin/ and usr.bin/ use the following idiom
> > to make sure that there are no options present:
> >  
> > while ((ch = getopt(argc, argv, "")) != -1)
> > switch (ch) {
> > case '?':
> > default:
> > usage();
> > /* NOTREACHED */
> > }
> >
> > if (argc != optind) {
> > usage();
> > /* NOTREACHED */
> > }
> >  
> > Why is this better then simply checking that (argc == 1)?
> 
> getopt(3) handles --.  Using getopt(3) everywhere is good for
> consistency.

I don't get it: why do we need to handle --
in utils which take no options and no arguments?

e.g. logname(1) is supposed to be launched just like "logname".
Does logname.c do the above just to handle "logname --" ?

Jan

Document ripd/ripctl -s

[Jeremie Courreges-Anglas]

As noted by Sebastien Leclerc in
http://marc.info/?l=openbsd-bugs=147560563030465=2

Copied almost verbatim from ospfd/ospfctl.  ok?


Index: usr.sbin/ripctl/ripctl.8
===
RCS file: /d/cvs/src/usr.sbin/ripctl/ripctl.8,v
retrieving revision 1.11
diff -u -p -r1.11 ripctl.8
--- usr.sbin/ripctl/ripctl.827 Jul 2015 18:48:05 -  1.11
+++ usr.sbin/ripctl/ripctl.812 Oct 2016 12:18:12 -
@@ -23,6 +23,7 @@
 .Nd control the Routing Information Protocol daemon
 .Sh SYNOPSIS
 .Nm
+.Op Fl s Ar socket
 .Ar command
 .Op Ar argument ...
 .Sh DESCRIPTION
@@ -35,6 +36,17 @@ Commands may be abbreviated to the minim
 .Cm s n
 for
 .Cm show neighbor .
+.Pp
+The following options are available:
+.Bl -tag -width Ds
+.It Fl s Ar socket
+Use
+.Ar socket
+instead of the default
+.Pa /var/run/ripd.sock
+to communicate with
+.Xr ripd 8 .
+.El
 .Pp
 The following commands are available:
 .Bl -tag -width Ds
Index: usr.sbin/ripd/ripd.8
===
RCS file: /d/cvs/src/usr.sbin/ripd/ripd.8,v
retrieving revision 1.12
diff -u -p -r1.12 ripd.8
--- usr.sbin/ripd/ripd.827 Jul 2015 17:28:40 -  1.12
+++ usr.sbin/ripd/ripd.812 Oct 2016 12:17:16 -
@@ -26,6 +26,7 @@
 .Op Fl dnv
 .Op Fl D Ar macro Ns = Ns Ar value
 .Op Fl f Ar file
+.Op Fl s Ar socket
 .Sh DESCRIPTION
 .Nm
 is the Routing Information Protocol
@@ -60,6 +61,8 @@ Specify an alternative configuration fil
 .It Fl n
 Configtest mode.
 Only check the configuration file for validity.
+.It Fl s Ar socket
+Use an alternate location for the default control socket.
 .It Fl v
 Produce more verbose output.
 .El


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: crontab(5): document -q flag in command

[Jeremie Courreges-Anglas]

Wouter Clarie  writes:

> The -q flag for the command in a crontab(5) entry was introduced in revision 
> 1.8 of src/usr.sbin/cron/entry.c back in 2001, but never documented.

Good catch.

> (Sorry, I'm no hero with mdoc, so not sure if the markup is correct.)

I'm no mdoc here either, but:
- I'd drop the first Pp, IMO -q should be described in the same
  paragraph as other special characters.
- I find it weird to use Fl to describe what is technically not
  a command-line option.  I went with Ql, just like for % and \.

Updated diff, assuming the point above are valid; input welcome.


Index: crontab.5
===
RCS file: /d/cvs/src/usr.sbin/cron/crontab.5,v
retrieving revision 1.33
diff -u -p -p -u -r1.33 crontab.5
--- crontab.5   30 Jan 2014 20:02:42 -  1.33
+++ crontab.5   12 Oct 2016 13:13:56 -
@@ -193,6 +193,11 @@ will be changed into newline characters,
 after the first
 .Ql %
 will be sent to the command as standard input.
+If the
+.Ar command
+field starts with
+.Ql -q ,
+execution will not be logged.
 .Pp
 Commands are executed by
 .Xr cron 8

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

systat(1) hostname

[Otto Moerbeek]

Hi,

simple diff to show the hostname on the second line. OK?

BTW, batch mode doesn't function here as expected. Need to look into that,

-Otto

Index: main.c
===
RCS file: /cvs/src/usr.bin/systat/main.c,v
retrieving revision 1.64
diff -u -p -r1.64 main.c
--- main.c  2 Jan 2016 15:02:05 -   1.64
+++ main.c  8 Oct 2016 13:34:07 -
@@ -124,8 +124,10 @@ print_header(void)
 
if (rawmode)
printf("\n\n%s\n", header);
-   else
+   else {
mvprintw(0, 0, "%s", header);
+   mvprintw(1, 1, "%s", hostname);
+   }
 
return (1);
 }

Re: enforce zero options

[Jeremie Courreges-Anglas]

Jan Stary  writes:

> Some programs in bin/ and usr.bin/ use the following idiom
> to make sure that there are no options present:
>  
>   while ((ch = getopt(argc, argv, "")) != -1)
>   switch (ch) {
>   case '?':
>   default:
>   usage();
>   /* NOTREACHED */
>   }
>
>   if (argc != optind) {
>   usage();
>   /* NOTREACHED */
>   }
>  
> Why is this better then simply checking that (argc == 1)?

getopt(3) handles --.  Using getopt(3) everywhere is good for
consistency.

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

PING: Re: logname turd polish

[Ingo Schwarze]

Hi Ted,

please just commit this patch you sent out back in July.
The old, verbose style keeps confusing people,
see for example Jan Stary's recent messages to tech@.
I guess you just overlooked my OK.
There was no opposition when you put this on tech@.

Yours,
  Ingo


Ingo Schwarze wrote on Fri, Jul 01, 2016 at 11:40:01PM +0200:
> Ted Unangst wrote on Thu, Jun 23, 2016 at 09:06:10PM -0400:

>> just because.

> OK schwarze@
>   Ingo
 

Index: logname.c
===
RCS file: /cvs/src/usr.bin/logname/logname.c,v
retrieving revision 1.9
diff -u -p -r1.9 logname.c
--- logname.c   9 Oct 2015 01:37:08 -   1.9
+++ logname.c   24 Jun 2016 01:02:58 -
@@ -32,45 +32,30 @@
 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 
-void usage(void);
+static void __dead
+usage(void)
+{
+   (void)fprintf(stderr, "usage: logname\n");
+   exit(1);
+}
 
 int
 main(int argc, char *argv[])
 {
-   int ch;
-   char *p;
-
-   setlocale(LC_ALL, "");
+   const char *p;
 
if (pledge("stdio", NULL) == -1)
err(1, "pledge");
 
-   while ((ch = getopt(argc, argv, "")) != -1)
-   switch (ch) {
-   case '?':
-   default:
-   usage();
-   /* NOTREACHED */
-   }
-
-   if (argc != optind) {
+   if (!(argc == 1 || (argc == 2 && strcmp(argv[1], "--") == 0)))
usage();
-   /* NOTREACHED */
-   }
 
if ((p = getlogin()) == NULL)
err(1, NULL);
(void)printf("%s\n", p);
-   exit(0);
-}
-
-void
-usage(void)
-{
-   (void)fprintf(stderr, "usage: logname\n");
-   exit(1);
+   return 0;
 }

switchd(8): implement the setconfig message

[Rafael Zalamena]

This diff teaches switchd(8) how to send the set_config message for
OpenFlow 1.3.5. We need this to set the default miss_send_len to
a value greater than zero so we can receive packets from the switch(4)
with the payload.

ok?


Index: ofp13.c
===
RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp13.c,v
retrieving revision 1.19
diff -u -p -r1.19 ofp13.c
--- ofp13.c 7 Oct 2016 08:49:53 -   1.19
+++ ofp13.c 12 Oct 2016 12:47:40 -
@@ -99,6 +99,12 @@ struct ofp_group_mod *
 struct ofp_bucket *
ofp13_bucket(struct ibuf *, uint16_t, uint32_t, uint32_t);
 
+int ofp13_setconfig_validate(struct switchd *,
+   struct sockaddr_storage *, struct sockaddr_storage *,
+   struct ofp_header *, struct ibuf *);
+int ofp13_setconfig(struct switchd *, struct switch_connection *,
+   uint16_t, uint16_t);
+
 struct ofp_callback ofp13_callbacks[] = {
{ OFP_T_HELLO,  ofp13_hello, NULL },
{ OFP_T_ERROR,  NULL, ofp13_validate_error },
@@ -109,7 +115,7 @@ struct ofp_callback ofp13_callbacks[] = 
{ OFP_T_FEATURES_REPLY, NULL, NULL },
{ OFP_T_GET_CONFIG_REQUEST, NULL, NULL },
{ OFP_T_GET_CONFIG_REPLY,   NULL, NULL },
-   { OFP_T_SET_CONFIG, NULL, NULL },
+   { OFP_T_SET_CONFIG, NULL, ofp13_setconfig_validate },
{ OFP_T_PACKET_IN,  ofp13_packet_in,
ofp13_validate_packet_in },
{ OFP_T_FLOW_REMOVED,   ofp13_flow_removed, NULL },
@@ -585,6 +591,8 @@ ofp13_hello(struct switchd *sc, struct s
OFP_TABLE_ID_ALL);
ofp13_table_features(sc, con, 0);
ofp13_desc(sc, con);
+   ofp13_setconfig(sc, con, OFP_CONFIG_FRAG_REASM,
+   OFP_CONTROLLER_MAXLEN_NO_BUFFER);
 
return (0);
 }
@@ -1461,4 +1469,48 @@ ofp13_bucket(struct ibuf *ibuf, uint16_t
b->b_watch_port = htonl(watchport);
b->b_watch_group = htonl(watchgroup);
return (b);
+}
+
+int
+ofp13_setconfig_validate(struct switchd *sc,
+struct sockaddr_storage *src, struct sockaddr_storage *dst,
+struct ofp_header *oh, struct ibuf *ibuf)
+{
+   struct ofp_switch_config*cfg;
+
+   if ((cfg = ibuf_seek(ibuf, 0, sizeof(*cfg))) == NULL)
+   return (-1);
+
+   log_debug("\tflags %#04x miss_send_len %d",
+   ntohs(cfg->cfg_flags), ntohs(cfg->cfg_miss_send_len));
+   return (0);
+}
+
+int
+ofp13_setconfig(struct switchd *sc, struct switch_connection *con,
+ uint16_t flags, uint16_t misslen)
+{
+   struct ibuf *ibuf;
+   struct ofp_switch_config*cfg;
+   struct ofp_header   *oh;
+   int  rv;
+
+   if ((ibuf = ibuf_static()) == NULL ||
+   (cfg = ibuf_advance(ibuf, sizeof(*cfg))) == NULL)
+   return (-1);
+
+   cfg->cfg_flags = htons(flags);
+   cfg->cfg_miss_send_len = htons(misslen);
+
+   oh = >cfg_oh;
+   oh->oh_version = OFP_V_1_3;
+   oh->oh_type = OFP_T_SET_CONFIG;
+   oh->oh_length = htons(ibuf_length(ibuf));
+   oh->oh_xid = htonl(con->con_xidnxt++);
+   if (ofp13_validate(sc, >con_local, >con_peer, oh, ibuf) != 0)
+   return (-1);
+
+   rv = ofp_output(con, NULL, ibuf);
+   ibuf_free(ibuf);
+   return (rv);
 }

Re: let head(1) understand `-' as stdin

[Jeremie Courreges-Anglas]

Ingo Schwarze  writes:

> Hi,
>
> Theo de Raadt wrote on Tue, Oct 11, 2016 at 01:35:34PM -0600:
>> jca@ wrote:
>>> Jan Stary  writes:
>
 The diff below makes head(1) recognize `-'
 as a name for the standard input,
 as many other utilities do.
>
>>> Makes sense to me.  The following points could be improved IMO:
>>> - using strcmp sounds cleaner than those char comparisons
>>> - I don't think the man page bits are needed.  Utilities that read from
>>>   stdin are supposed to support `-'.  I'm not sure whether the extra
>>>   example is really helpful.
>>> - should we avoid closing stdin (multiple times)?  Even though our
>>>   fclose(3) seems to cope with this, it seems that neither the
>>>   C standard nor POSIX offer such a guarantee.
>
>> Do standards permit that extension?
>
> POSIX neither requires nor forbids it, but encourages consistency
> among all the utilities taking [file ...] arguments within a given
> operating system:
>
>   http://pubs.opengroup.org/onlinepubs/9699919799/utilities/head.html
>
>   NAME
> head - copy the first part of files
>   OPTIONS
> The head utility shall conform to XBD Utility Syntax Guidelines.
>   STDIN
> The standard input shall be used if no file operands are
> specified, and shall be used if a file operand is '-' and the
> implementation treats the '-' as meaning standard input.
> Otherwise, the standard input shall not be used. See the INPUT
> FILES section.
>
>   http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html
>
>   Guideline 13:
> For utilities that use operands to represent files to be opened
> for either reading or writing, the '-' operand should be used
> to mean only standard input (or standard output when it is clear
> from context that an output file is being specified) or a file
> named -.
>
>   Where a utility described in the Shell and Utilities volume of
>   POSIX.1-2008 as conforming to these guidelines is required to
>   accept, or not to accept, the operand '-' to mean standard input
>   or output, this usage is explained in the OPERANDS section.
>   Otherwise, if such a utility uses operands to represent files,
>   it is implementation-defined whether the operand '-' stands for
>   standard input (or standard output), or for a file named -.
>
> (Enjoy language lawyers' paradise.)
>
>> This is command used in scripts.  Scripts are often portable.  If one
>> operating system has an extension, but others don't, then those
>> scripts become unportable to use use of these extensions.
>
>  * 1BSDfirst had head(1) (by Bill Joy 1977),
>of course treats "-" as a filename
>  * AT System V UNIX  didn't provide head(1) at all
>  * NetBSD  treats "-" as a filename
>  * FreeBSD treats "-" as a filename
>  * DragonFly   treats "-" as a filename
>  * illumos treats "-" as a filename
>  * Oracle Solaris 11   treats "-" as a filename
>  * GNU coreutils   treats "-" as standard input
>
> Some related utilities:
>
>tail(1)   grep(1)   sed(1)
>source: UNIX v7   UNIX v4   UNIX v7  (of course all filename)
>
>  * 4.4BSD-L2   filename  filename  filename
>  * System Vfilename  filename  filename
>  * OpenBSD filename  filename  filename
>  * NetBSD  filename  stdin filename
>  * FreeBSD filename  stdin filename
>  * DragonFly   filename  stdin filename
>  * illumos filename  filename  filename
>  * Solaris 11  stdin filename  filename
>  * GNU stdin stdin stdin
>
> cat(1), sort(1): POSIX requires treating "-" as standard input
>
>> I'm not raising a new argument here, it's been raised numerous times
>> when it comes to commands commonly used in scripts.
>> 
>> So consider that first.
>
> head(1) is firmly a BSD thingy, and all BSDs agree that "-" is a
> file name and not standard input.  POSIX explicitly encourages
> treating it as standard input ***if you do that for other utilities,
> too***, and GNU coreutils has the only head(1) implementation i
> found so far that actually does it.
>
> The bigger picture seems to be that OpenBSD and illumos tend to resist
> treating "-" as standard input whereever resisting is allowed,
> while GNU embraces treating "-" as standard whereever allowed.
> Most other systems seem to be somewhat inconsistent, in particular
> in those cases where they imported GNU utilities.
>
> So much for the facts.

Thanks a lot for this thorough analysis.

>
> I see two ways forward that make sense to me:
>
>  a) Either remain conservative - in line with both BSD and SysV
> heritage - and not do it unless required by the standard.
>
>  b) Or switch over to doing it whereever allowed - but then we
> should do it not just for head(1), but also for tail(1),
> grep(1), sed(1) and probably several others, and then we
> should probably also try to push such patches to 

Re: vmd/vmctl load/reload/reset

[Rafael Zalamena]

On Wed, Oct 12, 2016 at 02:06:35PM +0200, Reyk Floeter wrote:
> On Wed, Oct 12, 2016 at 01:44:25PM +0200, Reyk Floeter wrote:
> > Hi,
> > 
> > vmctl reload is currently broken, the attached diff fixes it and
> > re-introduces the semantics that originally came from iked:
> > 
> > - load/reload just reloads the configuration without clearing any
> > running configuration.  This way you can start vmd with a few
> > configured vms, terminate one vm, and reload the configuration which
> > will restart the terminated vm but keep the running ones.
> > 
> > - reset clears the configuration (and possibly terminates vms) without
> > reloading the configuration.  "vmctl reset" thus terminates all vms.
> > 
> > # vmctl load /etc/my-personal-vm.conf
> > # vmctl reload
> > # vmctl reset
> > 
> > OK?
> > 
> 
> ajacoutot@ reminded me of SIGHUP:
> change it to reload instead of reset on HUP.
> 
> Updated diff, OK?

It fixes my reload problem (vmctl and SIGHUP), "vmctl load " works,
"vmctl reset" also works and the code reads fine.

ok rzalamena@

Re: vmd/vmctl load/reload/reset

[Reyk Floeter]

On Wed, Oct 12, 2016 at 01:44:25PM +0200, Reyk Floeter wrote:
> Hi,
> 
> vmctl reload is currently broken, the attached diff fixes it and
> re-introduces the semantics that originally came from iked:
> 
> - load/reload just reloads the configuration without clearing any
> running configuration.  This way you can start vmd with a few
> configured vms, terminate one vm, and reload the configuration which
> will restart the terminated vm but keep the running ones.
> 
> - reset clears the configuration (and possibly terminates vms) without
> reloading the configuration.  "vmctl reset" thus terminates all vms.
> 
> # vmctl load /etc/my-personal-vm.conf
> # vmctl reload
> # vmctl reset
> 
> OK?
> 

ajacoutot@ reminded me of SIGHUP:
change it to reload instead of reset on HUP.

Updated diff, OK?

Reyk

Index: usr.sbin/vmctl/main.c
===
RCS file: /cvs/src/usr.sbin/vmctl/main.c,v
retrieving revision 1.17
diff -u -p -u -p -r1.17 main.c
--- usr.sbin/vmctl/main.c   10 May 2016 11:00:54 -  1.17
+++ usr.sbin/vmctl/main.c   12 Oct 2016 12:01:14 -
@@ -50,6 +50,8 @@ intvmm_action(struct parse_result *);
 int ctl_console(struct parse_result *, int, char *[]);
 int ctl_create(struct parse_result *, int, char *[]);
 int ctl_load(struct parse_result *, int, char *[]);
+int ctl_reload(struct parse_result *, int, char *[]);
+int ctl_reset(struct parse_result *, int, char *[]);
 int ctl_start(struct parse_result *, int, char *[]);
 int ctl_status(struct parse_result *, int, char *[]);
 int ctl_stop(struct parse_result *, int, char *[]);
@@ -57,8 +59,9 @@ intctl_stop(struct parse_result *, in
 struct ctl_command ctl_commands[] = {
{ "console",CMD_CONSOLE,ctl_console,"id" },
{ "create", CMD_CREATE, ctl_create, "\"path\" -s size", 1 },
-   { "load",   CMD_LOAD,   ctl_load,   "[path]" },
-   { "reload", CMD_RELOAD, ctl_load,   "[path]" },
+   { "load",   CMD_LOAD,   ctl_load,   "\"path\"" },
+   { "reload", CMD_RELOAD, ctl_reload, "" },
+   { "reset",  CMD_RESET,  ctl_reset,  "[all|vms|switches]" },
{ "start",  CMD_START,  ctl_start,  "\"name\""
" [-c] -k kernel -m size [-i count] [-d disk]*" },
{ "status", CMD_STATUS, ctl_status, "[id]" },
@@ -204,14 +207,18 @@ vmmaction(struct parse_result *res)
case CMD_CONSOLE:
get_info_vm(res->id, res->name, 1);
break;
+   case CMD_LOAD:
+   imsg_compose(ibuf, IMSG_VMDOP_LOAD, 0, 0, -1,
+   res->path, strlen(res->path) + 1);
+   done = 1;
+   break;
case CMD_RELOAD:
-   imsg_compose(ibuf, IMSG_VMDOP_RELOAD, 0, 0, -1,
-   res->path, res->path == NULL ? 0 : strlen(res->path) + 1);
+   imsg_compose(ibuf, IMSG_VMDOP_RELOAD, 0, 0, -1, NULL, 0);
done = 1;
break;
-   case CMD_LOAD:
-   imsg_compose(ibuf, IMSG_VMDOP_LOAD, 0, 0, -1,
-   res->path, res->path == NULL ? 0 : strlen(res->path) + 1);
+   case CMD_RESET:
+   imsg_compose(ibuf, IMSG_CTL_RESET, 0, 0, -1,
+   >mode, sizeof(res->mode));
done = 1;
break;
case CMD_CREATE:
@@ -431,16 +438,41 @@ ctl_status(struct parse_result *res, int
 int
 ctl_load(struct parse_result *res, int argc, char *argv[])
 {
-   char*config_file = NULL;
-
-   if (argc == 2)
-   config_file = argv[1];
-   else if (argc > 2)
+   if (argc != 2)
ctl_usage(res->ctl);
 
-   if (config_file != NULL &&
-   (res->path = strdup(config_file)) == NULL)
+   if ((res->path = strdup(argv[1])) == NULL)
err(1, "strdup");
+
+   return (vmmaction(res));
+}
+
+int
+ctl_reload(struct parse_result *res, int argc, char *argv[])
+{
+   if (argc != 1)
+   ctl_usage(res->ctl);
+
+   return (vmmaction(res));
+}
+
+int
+ctl_reset(struct parse_result *res, int argc, char *argv[])
+{
+   if (argc == 2) {
+   if (strcasecmp("all", argv[1]) == 0)
+   res->mode = CONFIG_ALL;
+   else if (strcasecmp("vms", argv[1]) == 0)
+   res->mode = CONFIG_VMS;
+   else if (strcasecmp("switches", argv[1]) == 0)
+   res->mode = CONFIG_SWITCHES;
+   else
+   ctl_usage(res->ctl);
+   } else if (argc > 2)
+   ctl_usage(res->ctl);
+
+   if (res->mode == 0)
+   res->mode = CONFIG_ALL;
 
return (vmmaction(res));
 }
Index: usr.sbin/vmctl/vmctl.8
===

Re: cdce(4): Remove zaurus specific code

[Mark Kettenis]

> Date: Tue, 11 Oct 2016 23:03:59 +0200
> From: Frederic Cambus 
> 
> On Fri, Oct 07, 2016 at 06:02:35PM +0200, Mark Kettenis wrote:
> 
> > > It seems there are still some leftovers from the zaurus port removal.
> > > 
> > > Comments? OK?
> > 
> > Not ok.  This is support for the zaurus as a usb device attached to an
> 
> My mistake, thanks for the clarification.
> 
> > OpenBSD machine.  Perhaps the option should be renamed (to CDCE_CRC32
> > for example) to make this clear.
> 
> That would make sense, what about the following diff?

Makes sesne to me. ok kettenis@

> Index: sys/dev/usb/if_cdce.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_cdce.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 if_cdce.c
> --- sys/dev/usb/if_cdce.c 26 Sep 2016 07:09:32 -  1.71
> +++ sys/dev/usb/if_cdce.c 11 Oct 2016 10:37:53 -
> @@ -93,13 +93,13 @@ static uint32_tcdce_crc32(const void *
>  const struct cdce_type cdce_devs[] = {
>  {{ USB_VENDOR_ACERLABS, USB_PRODUCT_ACERLABS_M5632 }, 0 },
>  {{ USB_VENDOR_PROLIFIC, USB_PRODUCT_PROLIFIC_PL2501 }, 0 },
> -{{ USB_VENDOR_SHARP, USB_PRODUCT_SHARP_SL5500 }, CDCE_ZAURUS },
> -{{ USB_VENDOR_SHARP, USB_PRODUCT_SHARP_A300 }, CDCE_ZAURUS },
> -{{ USB_VENDOR_SHARP, USB_PRODUCT_SHARP_SL5600 }, CDCE_ZAURUS },
> -{{ USB_VENDOR_SHARP, USB_PRODUCT_SHARP_C700 }, CDCE_ZAURUS },
> -{{ USB_VENDOR_SHARP, USB_PRODUCT_SHARP_C750 }, CDCE_ZAURUS },
> -{{ USB_VENDOR_MOTOROLA2, USB_PRODUCT_MOTOROLA2_USBLAN }, CDCE_ZAURUS },
> -{{ USB_VENDOR_MOTOROLA2, USB_PRODUCT_MOTOROLA2_USBLAN2 }, CDCE_ZAURUS },
> +{{ USB_VENDOR_SHARP, USB_PRODUCT_SHARP_SL5500 }, CDCE_CRC32 },
> +{{ USB_VENDOR_SHARP, USB_PRODUCT_SHARP_A300 }, CDCE_CRC32 },
> +{{ USB_VENDOR_SHARP, USB_PRODUCT_SHARP_SL5600 }, CDCE_CRC32 },
> +{{ USB_VENDOR_SHARP, USB_PRODUCT_SHARP_C700 }, CDCE_CRC32 },
> +{{ USB_VENDOR_SHARP, USB_PRODUCT_SHARP_C750 }, CDCE_CRC32 },
> +{{ USB_VENDOR_MOTOROLA2, USB_PRODUCT_MOTOROLA2_USBLAN }, CDCE_CRC32 },
> +{{ USB_VENDOR_MOTOROLA2, USB_PRODUCT_MOTOROLA2_USBLAN2 }, CDCE_CRC32 },
>  {{ USB_VENDOR_GMATE, USB_PRODUCT_GMATE_YP3X00 }, 0 },
>  {{ USB_VENDOR_NETCHIP, USB_PRODUCT_NETCHIP_ETHERNETGADGET }, 0 },
>  {{ USB_VENDOR_COMPAQ, USB_PRODUCT_COMPAQ_IPAQLINUX }, 0 },
> @@ -409,8 +409,8 @@ cdce_encap(struct cdce_softc *sc, struct
>   c = >cdce_cdata.cdce_tx_chain[idx];
>  
>   m_copydata(m, 0, m->m_pkthdr.len, c->cdce_buf);
> - if (sc->cdce_flags & CDCE_ZAURUS) {
> - /* Zaurus wants a 32-bit CRC appended to every frame */
> + if (sc->cdce_flags & CDCE_CRC32) {
> + /* Some devices want a 32-bit CRC appended to every frame */
>   u_int32_t crc;
>  
>   crc = cdce_crc32(c->cdce_buf, m->m_pkthdr.len);
> @@ -741,8 +741,8 @@ cdce_rxeof(struct usbd_xfer *xfer, void 
>   sc->cdce_rxeof_errors = 0;
>  
>   usbd_get_xfer_status(xfer, NULL, NULL, _len, NULL);
> - if (sc->cdce_flags & CDCE_ZAURUS)
> - total_len -= 4; /* Strip off CRC added by Zaurus */
> + if (sc->cdce_flags & CDCE_CRC32)
> + total_len -= 4; /* Strip off added CRC */
>   if (total_len <= 1)
>   goto done;
>  
> Index: sys/dev/usb/if_cdcereg.h
> ===
> RCS file: /cvs/src/sys/dev/usb/if_cdcereg.h,v
> retrieving revision 1.6
> diff -u -p -r1.6 if_cdcereg.h
> --- sys/dev/usb/if_cdcereg.h  4 Dec 2013 00:52:52 -   1.6
> +++ sys/dev/usb/if_cdcereg.h  11 Oct 2016 10:37:53 -
> @@ -41,7 +41,7 @@
>  struct cdce_type {
>   struct usb_devno cdce_dev;
>   u_int16_tcdce_flags;
> -#define CDCE_ZAURUS  1
> +#define CDCE_CRC32   1
>  #define CDCE_SWAPUNION   2
>  };
>  
> 

vmd/vmctl load/reload/reset

[Reyk Floeter]

Hi,

vmctl reload is currently broken, the attached diff fixes it and
re-introduces the semantics that originally came from iked:

- load/reload just reloads the configuration without clearing any
running configuration.  This way you can start vmd with a few
configured vms, terminate one vm, and reload the configuration which
will restart the terminated vm but keep the running ones.

- reset clears the configuration (and possibly terminates vms) without
reloading the configuration.  "vmctl reset" thus terminates all vms.

# vmctl load /etc/my-personal-vm.conf
# vmctl reload
# vmctl reset

OK?

Reyk

Index: usr.sbin/vmd/control.c
===
RCS file: /cvs/src/usr.sbin/vmd/control.c,v
retrieving revision 1.8
diff -u -p -u -p -r1.8 control.c
--- usr.sbin/vmd/control.c  29 Sep 2016 22:42:04 -  1.8
+++ usr.sbin/vmd/control.c  12 Oct 2016 11:37:21 -
@@ -367,6 +367,7 @@ control_dispatch_imsg(int fd, short even
break;
case IMSG_VMDOP_LOAD:
case IMSG_VMDOP_RELOAD:
+   case IMSG_CTL_RESET:
proc_forward_imsg(ps, , PROC_PARENT, -1);
break;
default:
Index: usr.sbin/vmd/vmd.c
===
RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
retrieving revision 1.33
diff -u -p -u -p -r1.33 vmd.c
--- usr.sbin/vmd/vmd.c  6 Oct 2016 18:48:41 -   1.33
+++ usr.sbin/vmd/vmd.c  12 Oct 2016 11:37:21 -
@@ -63,7 +63,8 @@ int
 vmd_dispatch_control(int fd, struct privsep_proc *p, struct imsg *imsg)
 {
struct privsep  *ps = p->p_ps;
-   int  res = 0, cmd = 0, v = 0;
+   int  res = 0, cmd = 0;
+   unsigned int v = 0;
struct vmop_create_paramsvmc;
struct vmop_id   vid;
struct vm_terminate_params   vtp;
@@ -103,15 +104,19 @@ vmd_dispatch_control(int fd, struct priv
case IMSG_VMDOP_GET_INFO_VM_REQUEST:
proc_forward_imsg(ps, imsg, PROC_VMM, -1);
break;
-   case IMSG_VMDOP_RELOAD:
-   v = 1;
case IMSG_VMDOP_LOAD:
-   if (IMSG_DATA_SIZE(imsg) > 0)
-   str = get_string((uint8_t *)imsg->data,
-   IMSG_DATA_SIZE(imsg));
-   vmd_reload(v, str);
+   IMSG_SIZE_CHECK(imsg, str); /* at least one byte for path */
+   str = get_string((uint8_t *)imsg->data,
+   IMSG_DATA_SIZE(imsg));
+   case IMSG_VMDOP_RELOAD:
+   vmd_reload(0, str);
free(str);
break;
+   case IMSG_CTL_RESET:
+   IMSG_SIZE_CHECK(imsg, );
+   memcpy(, imsg->data, sizeof(v));
+   vmd_reload(v, str);
+   break;
default:
return (-1);
}
@@ -441,7 +446,7 @@ vmd_configure(void)
 }
 
 void
-vmd_reload(int reset, const char *filename)
+vmd_reload(unsigned int reset, const char *filename)
 {
/* Switch back to the default config file */
if (filename == NULL || *filename == '\0')
@@ -449,12 +454,16 @@ vmd_reload(int reset, const char *filena
 
log_debug("%s: level %d config file %s", __func__, reset, filename);
 
-   if (reset)
-   config_setreset(env, CONFIG_ALL);
-
-   if (parse_config(filename) == -1) {
-   log_debug("%s: failed to load config file %s",
-   __func__, filename);
+   if (reset) {
+   /* Purge the configuration */
+   config_purge(env, reset);
+   config_setreset(env, reset);
+   } else {
+   /* Reload the configuration */
+   if (parse_config(filename) == -1) {
+   log_debug("%s: failed to load config file %s",
+   __func__, filename);
+   }
}
 }
 
Index: usr.sbin/vmd/vmd.h
===
RCS file: /cvs/src/usr.sbin/vmd/vmd.h,v
retrieving revision 1.28
diff -u -p -u -p -r1.28 vmd.h
--- usr.sbin/vmd/vmd.h  6 Oct 2016 18:48:41 -   1.28
+++ usr.sbin/vmd/vmd.h  12 Oct 2016 11:37:21 -
@@ -152,7 +152,7 @@ struct vmd {
 };
 
 /* vmd.c */
-voidvmd_reload(int, const char *);
+voidvmd_reload(unsigned int, const char *);
 struct vmd_vm *vm_getbyvmid(uint32_t);
 struct vmd_vm *vm_getbyid(uint32_t);
 struct vmd_vm *vm_getbyname(const char *);
Index: usr.sbin/vmd/vmm.c
===
RCS file: /cvs/src/usr.sbin/vmd/vmm.c,v
retrieving revision 1.50
diff -u -p -u -p -r1.50 vmm.c
--- usr.sbin/vmd/vmm.c  12 Oct 2016 06:56:54 -  1.50
+++ usr.sbin/vmd/vmm.c  12 Oct 2016 11:37:21 -
@@ -179,6 +179,7 @@ vmm_dispatch_parent(int fd, struct privs
   

Re: opencvs - use correct size when creating h_table

[Frederic Cambus]

On Wed, Jun 22, 2016 at 12:23:38PM +0200, Joris Vink wrote:

> Don't allocate the length of a pointer but rather the
> entire size of the struct hash_head data structure
> when creating the h_table array.

> RCS file: /cvs/src/usr.bin/cvs/hash.c,v
>
> - htable->h_table = xcalloc(hsize, sizeof(struct hash_head *));
> + htable->h_table = xcalloc(hsize, sizeof(struct hash_head));

Looks good to me.

OK fcambus@ if anyone wants to commit.

vxlan payload fixup

[Mike Belopuhov]

After some additional consideration I think I can improve
upon my previous diff.  Pull-up of the Ethernet header can
and should be made conditional: since an Ethernet header
requires 2 byte alignment, any position within a real life
mbuf is a valid one so unless there's not enough data in
the first mbuf, we shouldn't do anything.  This also puts
this m_pullup in line with the other ones in the tree in
terms of the preceeding length check.

Also, I believe I was wrong insisting on requiring explicit
ETHER_ALIGN alignment.  It's not really necessary if we're
going to pull-up anyways.

And finally I'm using "sizeof(struct ether_header)" instead
of the define for consistency with the rest of the code.

OK?


diff --git sys/net/if_vxlan.c sys/net/if_vxlan.c
index 034e651..f010e3e 100644
--- sys/net/if_vxlan.c
+++ sys/net/if_vxlan.c
@@ -577,11 +577,12 @@ vxlan_lookup(struct mbuf *m, struct udphdr *uh, int 
iphlen,
struct ifnet*ifp;
int  skip;
 #if NBRIDGE > 0
struct bridge_tunneltag *brtag;
 #endif
-   struct mbuf *m0;
+   struct mbuf *n;
+   int  off;
 
/* XXX Should verify the UDP port first before copying the packet */
skip = iphlen + sizeof(*uh);
if (m->m_pkthdr.len - skip < sizeof(v))
return (0);
@@ -634,12 +635,14 @@ vxlan_lookup(struct mbuf *m, struct udphdr *uh, int 
iphlen,
 
/* not found */
return (0);
 
  found:
-   if (m->m_pkthdr.len < skip + sizeof(struct ether_header))
+   if (m->m_pkthdr.len < skip + sizeof(struct ether_header)) {
+   m_freem(m);
return (EINVAL);
+   }
 
m_adj(m, skip);
ifp = >sc_ac.ac_if;
 
 #if NBRIDGE > 0
@@ -656,19 +659,26 @@ vxlan_lookup(struct mbuf *m, struct udphdr *uh, int 
iphlen,
m->m_flags &= ~(M_MCAST|M_BCAST);
 
 #if NPF > 0
pf_pkt_addr_changed(m);
 #endif
-   if ((m = m_pullup(m, sizeof(struct ether_header))) == NULL)
+   if ((m->m_len < sizeof(struct ether_header)) &&
+   (m = m_pullup(m, sizeof(struct ether_header))) == NULL)
return (ENOBUFS);
 
-   if (!ALIGNED_POINTER(mtod(m, caddr_t) + ETHER_HDR_LEN, uint32_t)) {
-   m0 = m;
-   m = m_dup_pkt(m0, ETHER_ALIGN, M_NOWAIT);
-   m_freem(m0);
-   if (m == NULL)
+   n = m_getptr(m, sizeof(struct ether_header), );
+   if (n == NULL) {
+   m_freem(m);
+   return (EINVAL);
+   }
+   if (!ALIGNED_POINTER(mtod(n, caddr_t) + off, uint32_t)) {
+   n = m_dup_pkt(m, ETHER_ALIGN, M_NOWAIT);
+   /* Dispose of the original mbuf chain */
+   m_freem(m);
+   if (n == NULL)
return (ENOBUFS);
+   m = n;
}
 
ml_enqueue(, m);
if_input(ifp, );
 

Re: hide iwn firmware error log

[Stefan Sperling]

On Tue, Oct 11, 2016 at 11:32:27PM +0200, Jan Stary wrote:
> On Oct 06 12:46:21, s...@stsp.name wrote:
> > Disable the detailed fatal firmware error log in iwn(4) by default.
> 
> These are my iwm errors of today
> on a Dell Latitude E5570.

> I sure don't know what to do with them,
> but I'm glad I can at least report them.
> If this goes in, which one of them will disappear?

iwm is not the same driver as iwn.

enforce zero options

[Jan Stary]

Some programs in bin/ and usr.bin/ use the following idiom
to make sure that there are no options present:
 
while ((ch = getopt(argc, argv, "")) != -1)
switch (ch) {
case '?':
default:
usage();
/* NOTREACHED */
}

if (argc != optind) {
usage();
/* NOTREACHED */
}
 
Why is this better then simply checking that (argc == 1)?

Below is a diff to logname as an example.
(Remove the pointless locale while there.)

Jan


Index: logname.c
===
RCS file: /cvs/src/usr.bin/logname/logname.c,v
retrieving revision 1.9
diff -u -p -r1.9 logname.c
--- logname.c   9 Oct 2015 01:37:08 -   1.9
+++ logname.c   12 Oct 2016 08:38:49 -
@@ -32,7 +32,6 @@
 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -44,23 +43,11 @@ main(int argc, char *argv[])
int ch;
char *p;
 
-   setlocale(LC_ALL, "");
-
if (pledge("stdio", NULL) == -1)
err(1, "pledge");
 
-   while ((ch = getopt(argc, argv, "")) != -1)
-   switch (ch) {
-   case '?':
-   default:
-   usage();
-   /* NOTREACHED */
-   }
-
-   if (argc != optind) {
+   if (argc != 1)
usage();
-   /* NOTREACHED */
-   }
 
if ((p = getlogin()) == NULL)
err(1, NULL);

remove locale from logname(1)

[Jan Stary]

Why does logname(1) need to setlocale?

Jan

Index: logname.c
===
RCS file: /cvs/src/usr.bin/logname/logname.c,v
retrieving revision 1.9
diff -u -p -r1.9 logname.c
--- logname.c   9 Oct 2015 01:37:08 -   1.9
+++ logname.c   12 Oct 2016 08:35:11 -
@@ -32,7 +32,6 @@
 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -43,8 +42,6 @@ main(int argc, char *argv[])
 {
int ch;
char *p;
-
-   setlocale(LC_ALL, "");
 
if (pledge("stdio", NULL) == -1)
err(1, "pledge");

Re: patch: netstat -P - mention allowkmem

[Alexander Bluhm]

On Tue, Oct 11, 2016 at 11:40:10PM -0400, David Hill wrote:
> Hello -
> 
> netstat -P now requires kern.allowkmem to be set.

OK bluhm@

> 
> Index: netstat.1
> ===
> RCS file: /cvs/src/usr.bin/netstat/netstat.1,v
> retrieving revision 1.79
> diff -u -p -r1.79 netstat.1
> --- netstat.1 1 Sep 2016 14:20:13 -   1.79
> +++ netstat.1 12 Oct 2016 03:37:53 -
> @@ -234,6 +234,14 @@ option, also print socket, domain and pr
>  Only the super-user can use the
>  .Fl P
>  option.
> +.Pp
> +The
> +.Fl P
> +option requires the ability to open
> +.Pa /dev/kmem
> +which may be restricted based upon the value of the
> +.Ar kern.allowkmem
> +.Xr sysctl 8 .
>  .It Fl p Ar protocol
>  Restrict the output to
>  .Ar protocol ,