Re: Remove function from less(1)

[Ted Unangst]

Ricardo Mestre wrote:
> There's no need to have a handrolled function for a simple compare that
> is just used once throughout all the source, or should it be kept for
> code readability?

>   if ((info->search_type & SRCH_NO_REGEX) == 0)
> - return (!is_null_pattern(info->compiled));
> + return (info->compiled != NULL);
>   return (info->text != NULL);

nope. this function is silly. why no is_null_text function for the next line?
applied. thanks.

Enable deep C-states on AMD CPUs

[Mark Kettenis]

Except for the really old ones, all AMD CPUs keep the APIC running in
low-power mode.  They don't have the ARAT flag that Intel CPUs have
though.  The diff below sets that flag on family 12h and later, such
that acpicpu(4) enables the deeper C states on these CPUs.  This
matches what Linux does.

Seems to work fine on the APU2, but this needs to be tested on as many
systems with AMD CPUs as possible.  Keep an eye out for machine
lockups when running with this diff.


Index: identcpu.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
retrieving revision 1.66
diff -u -p -r1.66 identcpu.c
--- identcpu.c  13 Nov 2015 07:52:20 -  1.66
+++ identcpu.c  21 Nov 2015 16:27:03 -
@@ -545,12 +545,15 @@ identifycpu(struct cpu_info *ci)
printf(",%s", cpu_seff0_ecxfeatures[i].str);
}
 
-   if (!strcmp(cpu_vendor, "GenuineIntel") && cpuid_level >= 0x06 ) {
+   if (!strcmp(cpu_vendor, "GenuineIntel") && cpuid_level >= 0x06) {
CPUID(0x06, ci->ci_feature_tpmflags, dummy, dummy, dummy);
for (i = 0; i < nitems(cpu_tpm_eaxfeatures); i++)
if (ci->ci_feature_tpmflags &
cpu_tpm_eaxfeatures[i].bit)
printf(",%s", cpu_tpm_eaxfeatures[i].str);
+   } else if (!strcmp(cpu_vendor, "AuthenticAMD")) {
+   if (ci->ci_family >= 0x12)
+   ci->ci_feature_tpmflags |= TPM_ARAT;
}
 
printf("\n");

Re: PC Engines APU2 coming soon - how is OpenBSD's support so far?

[Mathias Schmocker]

Le 16.11.15 18:42, Jason Barbier a écrit :

Looks like you need to email support and get the updated BIOS. I had the
same problem and there is an update to the bios to allow for mSATA boot
but not SD card boot yet.


Yes, mSATA install and boot works after this bios update:
  http://pcengines.ch/file/apu151109.zip

  SeaBIOS (version rel-1.8.0-190-gc029eab-20151109_141843-wim-ws)
  BUILD: gcc:  binutils: (GNU Binutils) 2.23.2
  <...>
  Google, Inc.
  Serial Graphics Adapter 08/22/15
  SGABIOS $Id: sgabios.S 8 2010-04-22 00:03:40Z nlaredo $ 
(wiv@coreboot-Virtual-Machine) Sat Aug 22 09:25:30 UTC 2015


But hooking a 2TB WD Element HDD still hangs the APU2 on the next reboot 
(4k sectors, USB 3.0, bad init timings in the bios ?)


Symptoms:
  Searching bootorder for: /pci@i0cf8/*@11/drive@0/disk@0
  WARNING - Timeout at wait_bit:310!
  <...>
  USB MSC vendor='WD' product='Elements 10B8' rev='1012' type=0 removable=0
  call16 with invalid stack

And if you choose the memtest payload, there is no way to abort it.

Mathias

On Sat, Nov 14, 2015, at 10:22 AM, Mathias Schmocker wrote:
> Le 14.11.15 17:02, Chris Cappuccio a écrit :
>  > Mathias Schmocker [s...@smat.ch] wrote:
>  >>
>  >> First tests with a bootable OpenBSD amd64 5.8-current USB stick and
>  >> installation on the 16GB mSata internal storage.
>  >> After reboot, BIOS could find mSata to boot on, but defaulted to the
> memtest
>  >> boot payload
>  >>
>  >
>  > This is a setting you can change, although the default boot order
> would put memtest last.
>
> Not for me:
>
> 1. USB MSC Drive SanDisk Ultra Fit 1.00
> 2. Payload [memtest]
> 3. Payload [setup]
>
> I have now successfully installed and booted on a small 16GB SanDisk USB
> key, root on sd1a (but I had a hard lockup once, with >
> xhci_wait_for_command: Command ring still running ...)
>
> But pressing F10 and 3 to access setup and placing mSata in front of all
> the other does NOT work (even if sd0a has a bootable bsd.rd, /boot and
> the proper /etc/boot.conf, and is accessible once OpenBSD runs out of
> the USB key ...)
>
> Which SeaBIOS, BUILD version, and option rom does your APU2 display when
> booting up ?
>
>  > I've had no problems with apu2 detecting msata so far..
>  >
>  >> Unable to abort memtest by pressing (esc) on the serial console, or
> (c) for
>  >> configure.
>  >>
>  >
>  > You have to escape to the prompt before memtest starts.
>  >
>  >
>

int -> size_t in ksh

[Michael McConville]

It seems apparent that they never go negative. All instances are used to
manipulate strlen() values.

ok?


Index: c_ksh.c
===
RCS file: /cvs/src/bin/ksh/c_ksh.c,v
retrieving revision 1.46
diff -u -p -r1.46 c_ksh.c
--- c_ksh.c 21 Oct 2015 15:20:37 -  1.46
+++ c_ksh.c 22 Nov 2015 01:35:34 -
@@ -67,7 +67,7 @@ c_cd(char **wp)
}
} else if (!wp[2]) {
/* Two arguments - substitute arg1 in PWD for arg2 */
-   int ilen, olen, nlen, elen;
+   size_t ilen, olen, nlen, elen;
char *cp;
 
if (!current_wd[0]) {
@@ -1182,7 +1182,8 @@ c_kill(char **wp)
shprintf("%s%s", p, sigtraps[i].name);
shprintf("\n");
} else {
-   int w, i;
+   int i;
+   size_t w;
int mess_width;
struct kill_info ki;
 

Re: [patch] cwm: Preserve stacking order during cycling

[Артур Истомин]

On Sun, Nov 22, 2015 at 12:58:42AM +0100, Vadim Vygonets wrote:
> Quoth Артур Истомин on Sun, Nov 22, 2015:
> > On Sat, Nov 21, 2015 at 02:10:15AM +0100, Vadim Vygonets wrote:
> > > After cycling through many windows, the original window may be
> > > obscured by many others, and if you still want to see its
> > > contents you end up doing the Alt-Tab-Tab-Tab-Tab-Tab, Alt-Tab,
> > > Alt-Tab dance.
> > 
> > Even this not always helps. Often I need press Alt+Down or Alt+Up
> > to get window up.
> 
> Oh, I haven't seen it happening (but then I'm a new user).
> Sounds weird.  If you have a way to reproduce it, I'd like to
> know (although I'm reluctant to promise to work on it).

It is always reproducable for me when 3 or more windows opened. This
is not immediately, but eventualy I can't access to the window located
below one or two other through Alt+Tab. In such case Alt+Down rush to 
rescue :)

It is very annoying, that Alt+Tab in cwm does not work in "classic"
way, cycling ALL windows in order. But I've never seen someone complain
and thought it was my local problem. You became the first ray of hope :)

I'll try your patch in 10-12 hours after some sleep. 

Re: [patch] cwm: Preserve stacking order during cycling

[Артур Истомин]

On Sat, Nov 21, 2015 at 02:10:15AM +0100, Vadim Vygonets wrote:
> Hi,
> 
> After cycling through many windows, the original window may be
> obscured by many others, and if you still want to see its
> contents you end up doing the Alt-Tab-Tab-Tab-Tab-Tab, Alt-Tab,
> Alt-Tab dance.

Even this not always helps. Often I need press Alt+Down or Alt+Up
to get window up.

Thanks for your effort. 

> 
> This patch adds restacking of windows during cycling.  Hold Alt,
> press Tab and a window will be raised.  Press Tab again while
> still holding Alt and that window will be lowered back before
> another is raised.  Once you release Alt, the original window
> will be hidden behind no more than one other (the target),
> assuming it was raised before.
> 
> What do you think?
> 
> Vadik.
> 
> -- 
> Nondeterminism means never having to say you are wrong.

> ? cwm-incresize.diff
> ? p
> Index: calmwm.h
> ===
> RCS file: /cvs/xenocara/app/cwm/calmwm.h,v
> retrieving revision 1.311
> diff -u -r1.311 calmwm.h
> --- calmwm.h  12 Nov 2015 21:28:03 -  1.311
> +++ calmwm.h  21 Nov 2015 00:12:09 -
> @@ -62,6 +62,8 @@
>  #define CWM_CLIENT_RCYCLE0x0002
>  #define CWM_CLIENT_CYCLE_INGRP   0x0004
>  
> +#define CWM_CLIENT_RESTACK_GRP   0x0001
> +
>  #define CWM_CLIENT_TILE_HORIZ0x0001
>  #define CWM_CLIENT_TILE_VERT 0x0002
>  
> @@ -385,6 +387,7 @@
>  void  client_applysizehints(struct client_ctx *);
>  void  client_config(struct client_ctx *);
>  struct client_ctx*client_current(void);
> +void  client_restack(struct client_ctx_q *, int);
>  void  client_cycle(struct screen_ctx *, int);
>  void  client_cycle_leave(struct screen_ctx *);
>  void  client_delete(struct client_ctx *);
> Index: client.c
> ===
> RCS file: /cvs/xenocara/app/cwm/client.c,v
> retrieving revision 1.214
> diff -u -r1.214 client.c
> --- client.c  12 Nov 2015 18:33:30 -  1.214
> +++ client.c  21 Nov 2015 00:12:09 -
> @@ -664,6 +664,50 @@
>  }
>  
>  void
> +client_restack(struct client_ctx_q *clientq, int flags)
> +{
> +#define CLIENTQ_FOREACH(var, head, ingrp)\
> + for((var) = TAILQ_FIRST(head);  \
> + (var) != TAILQ_END(head);   \
> + (var) = (ingrp) ?   \
> + TAILQ_NEXT(var, group_entry) : TAILQ_NEXT(var, entry))
> + struct client_ctx   *cc;
> + Window  *winlist;
> + int  i, lastempty = -1;
> + int  nwins = 0, highstack = 0;
> +
> + CLIENTQ_FOREACH(cc, clientq, flags & CWM_CLIENT_RESTACK_GRP) {
> + if (cc->flags & CLIENT_HIDDEN)
> + continue;
> + if (cc->stackingorder > highstack)
> + highstack = cc->stackingorder;
> + }
> + winlist = xreallocarray(NULL, (highstack + 1), sizeof(*winlist));
> +
> + /* Invert the stacking order for XRestackWindows(). */
> + CLIENTQ_FOREACH(cc, clientq, flags & CWM_CLIENT_RESTACK_GRP) {
> + if (cc->flags & CLIENT_HIDDEN)
> + continue;
> + winlist[highstack - cc->stackingorder] = cc->win;
> + nwins++;
> + }
> +
> + /* Un-sparseify */
> + for (i = 0; i <= highstack; i++) {
> + if (!winlist[i] && lastempty == -1)
> + lastempty = i;
> + else if (winlist[i] && lastempty != -1) {
> + winlist[lastempty] = winlist[i];
> + if (++lastempty == i)
> + lastempty = -1;
> + }
> + }
> +
> + XRestackWindows(X_Dpy, winlist, nwins);
> + free(winlist);
> +}
> +
> +void
>  client_cycle(struct screen_ctx *sc, int flags)
>  {
>   struct client_ctx   *newcc, *oldcc;
> @@ -704,9 +748,15 @@
>   }
>   }
>  
> - /* reset when cycling mod is released. XXX I hate this hack */
> - sc->cycling = 1;
>   client_ptrsave(oldcc);
> + if (!sc->cycling) {
> + /* reset when cycling mod is released. XXX I hate this hack */
> + sc->cycling = 1;
> + screen_updatestackingorder(sc);
> + } else {
> + client_restack(>clientq, (flags & CWM_CLIENT_CYCLE_INGRP) ?
> + CWM_CLIENT_RESTACK_GRP : 0);
> + }
>   client_ptrwarp(newcc);
>  }
>  
> Index: group.c
> ===
> RCS file: /cvs/xenocara/app/cwm/group.c,v
> retrieving revision 1.121
> diff -u -r1.121 group.c
> --- group.c   10 Nov 2015 20:05:33 -  1.121
> +++ group.c   21 Nov 2015 00:12:09 -
> @@ -34,7 +34,6 @@
>  
>  static struct group_ctx  

Re: WAPBL implementation

[Michael McConville]

I agree that this is very cool. It's probably also worth mentioning that
there's a long discussion on NetBSD's tech-kernel@ right now about a
WABPL-related panic. Not sure whether that's relevant to this diff.

Re: WAPBL implementation

[Karel Gardas]

Hi Walter,

please scratch that. The email was written based on very short
experience with wapbl on a small 10gb partition. Now I've performed
testing on full 500gb partition all tests done using checksumming SR
RAID1. The tests differ only in mount option.

rsync: 12m15s (default) -> 7m40s (softdep) -> 6m35s (async) -> 6m5s (wapbl)
rm: 5m35s (default) -> 48s (softdep) -> 14s (wapbl) -> 13s (async)

so again, fantastic results. To kind of excuse me, I need to add that
my fear was caused by seeing huge numbers of small data written using
wapbl. After rsync/find/rm cycle I usually detach SR RAID1 drive and
this prints some statistic for me. For wapbl based run it looks:

RAID1C write statistics in format len (counter/collisions): 512
(917/917), 8192 (75667/293919), 65536 (10941/0), 32768 (204849/0),
4096 (593479/3174400), 12288 (33279/95801), 17408 (88/88), 64512
 (279/279), 20480 (12702/21085), 16384 (19738/43332), 28672
(7026/7636), 24576 (9129/11694), 50176 (7/7), 43520 (1/1), 22016
(2/2), 37888 (5/5), 27136 (2/2), 31744 (5/5), 61952 (5/5), 27648
(4/4), 14848 (4/4), 3072 (8/8), 29184 (6/6), 36352 (2/2), 10240 (3/3),
33280 (3/3), 32256 (4/4), 62976 (3/3), 53760 (8/8), 11776 (3/3), 43008
(5/5), 25088 (3/3), 40448 (6/6), 17920 (4/4), 41472 (3/3), 54784
(2/2), 55296 (3/3), 47104 (3/3), 18432 (12/12), 41984 (5/5), 10752
(2/2), 59904 (3/3), 33792 (1/1), 37376 (2/2), 30208 (1/1), 35328
(1/1), 19456 (6/6), 46080 (2/2), 22528 (6/6), 13312 (6/6), 49152
(5/5), 5120 (3/3), 23552 (5/5), 26112 (3/3), 51200 (7/7), 6144 (6/6),
59392 (4/4), 1024 (1/1), 9728 (2/2), 15872 (5/5), 38912 (1/1), 11264
(4/4), 18944 (29/29), 51712 (27/27), 52224 (5/5), 56320 (6/6), 9216
(4/4), 25600 (5/5), 39936 (5/5), 61440 (5/5), 35840 (3/3), 60928
(4/4), 4608 (2/2), 1536 (3/3), 47616 (3/3), 44032 (2/2), 57856 (3/3),
7680 (2/2), 3584 (2/2), 65024 (2/2), 15360 (2/2), 44544 (2/2), 5632
(4/4), 2560 (2/2), 58880 (3/3), 56832 (1/1), 49664 (2/2), 24064 (2/2),
60416 (4/4), 45056 (3/3), 40960 (3/3), 14336 (3/3), 36864 (2/2), 26624
(1/1), 42496 (4/4), 23040 (4/4), 12800 (4/4), 52736 (2/2), 55808
(2/2), 7168 (7/7), 58368 (6/6), 63488 (2/2), 34816 (1/1), 50688 (2/2),
31232 (3/3), 34304 (2/2), 2048 (1/1), 20992 (1/1), 48128 (3/3), 8704
(1/1), 53248 (3/3), 64000 (2/2), 62464 (3/3), 46592 (3/3), 6656 (2/2),
13824 (2/2), 45568 (2/2), 30720 (5/5), 39424 (2/2), 28160 (3/3), 29696
(1/1), 16896 (2/2),
RAID1C read statistics in format len (counter): 512 (62), 32768
(826676), 8192 (28), 12288 (14), 4096 (14), 65536 (18086),

Please note that collision, where there are not 0, you need to
subtract from the collision number the number of I/Os. This is slight
issue in my statistics collecting. From the log you can see that all
"unusal" operation shows number as (X/X) which means there was not
collision and the write was done in a slow way as read chksum block,
write data, write chksum block.

FYI: statistics from async run looks:

RAID1C write statistics in format len (counter/collisions): 512 (2/2),
8192 (75645/293154), 65536 (1154/0), 32768 (203966/0), 4096
(576745/3047106), 12288 (32958/95767), 16384 (19722/43407), 2048
0 (12696/21230), 28672 (7020/7591), 24576 (9125/11989),
RAID1C read statistics in format len (counter): 512 (75), 2048 (4),
32768 (826693), 8192 (32), 12288 (16), 4096 (16), 65536 (18166),

so you can see that wapbl do all sorts of unusal data lenght
operations, but does not hurt SR RAID1C by excess number of collisions
with different I/O. You can see such excess number of collisions in
case of 4096 bytes lenght write I/O: 4096 (593479/3174400).

So well, wapbl looks fantastic so far. I'm now hammering this on top
of SR RAID1C and will let you know about any issues...

Thanks!
Karel

On Sat, Nov 21, 2015 at 12:41 PM, Karel Gardas  wrote:
> RAID1. So here is my question: is there any possibility to convince
> current WAPBL code to write transasction into log in 32k blocks with
> 32k alignment? I can of course hack the code if you advice where to
> test that, I've just so far not been able to find the magic constant
> of commit size or so.

Re: [patch] cwm: Preserve stacking order during cycling

[Vadim Vygonets]

Quoth Артур Истомин on Sun, Nov 22, 2015:
> On Sat, Nov 21, 2015 at 02:10:15AM +0100, Vadim Vygonets wrote:
> > After cycling through many windows, the original window may be
> > obscured by many others, and if you still want to see its
> > contents you end up doing the Alt-Tab-Tab-Tab-Tab-Tab, Alt-Tab,
> > Alt-Tab dance.
> 
> Even this not always helps. Often I need press Alt+Down or Alt+Up
> to get window up.

Oh, I haven't seen it happening (but then I'm a new user).
Sounds weird.  If you have a way to reproduce it, I'd like to
know (although I'm reluctant to promise to work on it).

> Thanks for your effort. 

You're very welcome, world :)

Vadik.

-- 
Avoid reality at all costs.

Re: [patch] cwm: Preserve stacking order during cycling

[Vadim Vygonets]

Quoth Vadim Vygonets on Sat, Nov 21, 2015:
> This patch

has a bug, but it's late and I'm drunk.  Tomorrow I'll be
coherent enough not only to explain it, but to fix it too.  Good
day to all!

Vadik.

-- 
Who needs friends when you can sit alone in your room and drink?

Re: WAPBL implementation

[Karel Gardas]

On Sat, Nov 21, 2015 at 8:28 PM, Michael McConville  wrote:
> I agree that this is very cool. It's probably also worth mentioning that
> there's a long discussion on NetBSD's tech-kernel@ right now about a
> WABPL-related panic. Not sure whether that's relevant to this diff.

If I've found the same discussion then this looks like an overflow of
journal file. One recommended way to duplicate this issue is removing
"a tree". My test currently rsync /usr/src in 7 parallel processes
into /raid/1../raid/7 dirs, calculates sha1, unmount, mount, check
sha1 and finally rm whole /raid -- which honestly can be considered as
"remove the tree" operation I guess. Let's see if the thing panics for
me.

Re: PC Engines APU2 coming soon - how is OpenBSD's support so far?

[Stuart Henderson]

This is interesting but I think it's straying a bit off-topic for tech@ -
I think misc would be alright for general information about the machine,
though things which are clearly problems with the hw/bios (like not being
able to exit memtest - this is the same on APU1 btw) should go to the vendor.

Re: WAPBL implementation

[Karel Gardas]

Walter,

this is really beautiful at least from the performance point of view.
My benchmarks shows:

rsync: 3m27s -> 1m20s
rm: 2m13s -> 9s

so speed increase is here as reported by various wapbl papers which is
really nice.

Anyway, I'd also like to use that with my implementation of
checksumming for SR RAID1. I'm not that lucky like you and under some
conditions my code really sucks performance wise on write operation.
This is due to design/layout of chksumming but even if I experiment
with different layouts (or caching) it still kind of sucks on write.
The only way where it does not suck is if the data are written in 32k
multiplies and aligned on 32k boundary. This way my implementation is
running optimized way hence for write needing only to override chksum
block and write data. And this is done w/o collision with other I/O so
this way is fast and kind of acceptable in comparison with pure SR
RAID1. So here is my question: is there any possibility to convince
current WAPBL code to write transasction into log in 32k blocks with
32k alignment? I can of course hack the code if you advice where to
test that, I've just so far not been able to find the magic constant
of commit size or so.

Thanks!
Karel
PS: for curious my benchmarking shows those numbers. Benchmarked on 2x
500GB WD Re drives, 512 bytes sector size physical. Using my patch to
speedup CRC32 calculation using "by-four" version of algorithm
(360->970 MB/s speed increase). Comparison of pure SR RAID1 with my
chksumming SR RAID1 implementation on a 500GB fs (whole SR RAID
drive). Rsync is copying /usr/src into /raid (1.3GB of data (it's a
git repo) so bigger than usual CVS checkout), rm just rm /raid/* and
find: find /raid -type f cat \{} >/dev/null \;

rsync: 3m27s -> 12m15s
rm: 2m13s -> 5m35s
find+cat: 1m58s -> 2m10s (read benchmark, does not suck IMHO)
dd 2GB write: 16s -> 23s (shows potential of write 32k aligned, for me
very acceptable result)
dd 2GB read: 16s -> 21s

tcp_mss() fix

[Martin Pieuchot]

David reported a possible used of uninitialized ``ifp'' in tcp_mss().
This cannot currently happen but I think the diff below should prevent
a bad dereference later.

Ok?

Index: netinet/tcp_input.c
===
RCS file: /cvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.309
diff -u -p -r1.309 tcp_input.c
--- netinet/tcp_input.c 20 Nov 2015 10:45:29 -  1.309
+++ netinet/tcp_input.c 21 Nov 2015 11:51:53 -
@@ -2988,6 +2988,10 @@ tcp_mss(struct tcpcb *tp, int offer)
if (rt == NULL)
goto out;
 
+   ifp = if_get(rt->rt_ifidx);
+   if (ifp == NULL)
+   goto out;
+
switch (tp->pf) {
 #ifdef INET6
case AF_INET6:
@@ -3002,7 +3006,6 @@ tcp_mss(struct tcpcb *tp, int offer)
goto out;
}
 
-   ifp = if_get(rt->rt_ifidx);
/*
 * if there's an mtu associated with the route and we support
 * path MTU discovery for the underlying protocol family, use it.
@@ -3024,13 +3027,6 @@ tcp_mss(struct tcpcb *tp, int offer)
mss = rt->rt_rmx.rmx_mtu - iphlen -
sizeof(struct tcphdr);
}
-   } else if (ifp == NULL) {
-   /*
-* ifp may be null and rmx_mtu may be zero in certain
-* v6 cases (e.g., if ND wasn't able to resolve the
-* destination host.
-*/
-   goto out;
} else if (ifp->if_flags & IFF_LOOPBACK) {
mss = ifp->if_mtu - iphlen - sizeof(struct tcphdr);
} else if (tp->pf == AF_INET) {

Re: [patch] cwm: Preserve stacking order during cycling

[Martijn van Duren]

I use cwm on all my desktops and I really love its simplicity, but this 
has been one of the minor things that annoyed me.
I tried the patch and it worked like a charm. So as an active user, 
feature-wise: Yes please. :)


On 11/21/15 02:10, Vadim Vygonets wrote:

Hi,

After cycling through many windows, the original window may be
obscured by many others, and if you still want to see its
contents you end up doing the Alt-Tab-Tab-Tab-Tab-Tab, Alt-Tab,
Alt-Tab dance.

This patch adds restacking of windows during cycling.  Hold Alt,
press Tab and a window will be raised.  Press Tab again while
still holding Alt and that window will be lowered back before
another is raised.  Once you release Alt, the original window
will be hidden behind no more than one other (the target),
assuming it was raised before.

What do you think?

Vadik.

Re: rt_ifp and pf(4)

[Martin Pieuchot]

On 20/11/15(Fri) 18:05, Alexandr Nedvedicky wrote:
> Hello,
> 
> I have just nit comments, feel free to ignore them.
> 
> 1) would it be possible to use closing #endif guards as follows:
>   #endif  /* NCARP */

Done.

> 2) another nit here:
> > @@ -5607,6 +5616,8 @@ pf_route(struct mbuf **m, struct pf_rule
> >  done:
> > if (r->rt != PF_DUPTO)
> > *m = NULL;
> > +   if (!r->rt)
> > +   if_put(ifp);
> > rtfree(rt);
> > return;
> >  
> 
> I would probably use test as follows:
>   if (rt != NULL)
>   if_put(ifp);
> 
> it's detail, given at some point in future we will probably have to use
> if_put()/if_get() for ifp's bound to kif's (right?).

Well not necessarily.  kif have the same lifetime as an ifp and are easy
to garbage collect so there's no need for the moment to use index for
them.  Indexes are useful when two objects having a different lifetime
need to be linked at some point.  By using indexes/id/cookie we do not 
increase the lifetime of an object like with references but this has the
cost of a layer of indirection.

Re: calendar: add getpw promise

[Pascal Stumpf]

On Sat, 21 Nov 2015 13:18:47 +0100, Sebastien Marie wrote:
> Hi,
> 
> Here a patch that should correct a pledge kill in calendar.
> 
> calendar(1) use getpwent, so it needs "getpw" for running in YP
> environment.
> 
> OK ?
> -- 
> Sebastien Marie
> 
> Index: calendar.c
> ===
> RCS file: /cvs/src/usr.bin/calendar/calendar.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 calendar.c
> --- calendar.c16 Nov 2015 17:49:35 -  1.33
> +++ calendar.c21 Nov 2015 12:16:18 -
> @@ -119,11 +119,11 @@ main(int argc, char *argv[])
>   usage();
>  
>   if (doall) {
> - if (pledge("stdio rpath tmppath fattr id proc exec", NULL)
> + if (pledge("stdio rpath tmppath fattr getpw id proc exec", NULL)
>   == -1)
>   err(1, "pledge");
>   } else {
> - if (pledge("stdio rpath proc exec", NULL) == -1)
> + if (pledge("stdio rpath getpw proc exec", NULL) == -1)

The getpwent() call is below if (doall), so I think this is not needed.

>   err(1, "pledge");
>   }
>  
> 
> 

Remove function from less(1)

[Ricardo Mestre]

There's no need to have a handrolled function for a simple compare that
is just used once throughout all the source, or should it be kept for
code readability?

Index: funcs.h
===
RCS file: /cvs/src/usr.bin/less/funcs.h,v
retrieving revision 1.13
diff -u -p -u -r1.13 funcs.h
--- funcs.h 13 Nov 2015 16:48:48 -  1.13
+++ funcs.h 21 Nov 2015 12:12:58 -
@@ -239,7 +239,6 @@ extern  void ierror(const char *, PARG *)
 extern int query(const char *, PARG *);
 extern int compile_pattern(char *, int, regex_t **);
 extern void uncompile_pattern(regex_t **);
-extern int is_null_pattern(void *);
 extern int match_pattern(void *, char *, char *, int, char **, char **,
 int, int);
 extern off_t position(int);
Index: search.c
===
RCS file: /cvs/src/usr.bin/less/search.c,v
retrieving revision 1.13
diff -u -p -u -r1.13 search.c
--- search.c9 Nov 2015 16:39:13 -   1.13
+++ search.c21 Nov 2015 12:13:09 -
@@ -177,7 +177,7 @@ static int
 prev_pattern(struct pattern_info *info)
 {
if ((info->search_type & SRCH_NO_REGEX) == 0)
-   return (!is_null_pattern(info->compiled));
+   return (info->compiled != NULL);
return (info->text != NULL);
 }

Index: pattern.c
===
RCS file: /cvs/src/usr.bin/less/pattern.c,v
retrieving revision 1.8
diff -u -p -u -r1.8 pattern.c
--- pattern.c   9 Nov 2015 16:39:13 -   1.8
+++ pattern.c   21 Nov 2015 12:13:20 -
@@ -74,15 +74,6 @@ uncompile_pattern(regex_t **pattern)
 }

 /*
- * Is a compiled pattern null?
- */
-int
-is_null_pattern(void *pattern)
-{
-   return (pattern == NULL);
-}
-
-/*
  * Simple pattern matching function.
  * It supports no metacharacters like *, etc.
  */

calendar: add getpw promise

[Sebastien Marie]

Hi,

Here a patch that should correct a pledge kill in calendar.

calendar(1) use getpwent, so it needs "getpw" for running in YP
environment.

OK ?
-- 
Sebastien Marie

Index: calendar.c
===
RCS file: /cvs/src/usr.bin/calendar/calendar.c,v
retrieving revision 1.33
diff -u -p -r1.33 calendar.c
--- calendar.c  16 Nov 2015 17:49:35 -  1.33
+++ calendar.c  21 Nov 2015 12:16:18 -
@@ -119,11 +119,11 @@ main(int argc, char *argv[])
usage();
 
if (doall) {
-   if (pledge("stdio rpath tmppath fattr id proc exec", NULL)
+   if (pledge("stdio rpath tmppath fattr getpw id proc exec", NULL)
== -1)
err(1, "pledge");
} else {
-   if (pledge("stdio rpath proc exec", NULL) == -1)
+   if (pledge("stdio rpath getpw proc exec", NULL) == -1)
err(1, "pledge");
}
 

[patch] Fix tput(1) capability's needed argument count

[Alessandro DE LAURENZIS]

Dear tech@,

This is an attempt to solve a problem I previously reported [1] to misc@
(without receiving any feedback, so far).

>From terminfo(5), I understand that the stack mechanism is the only way
to parametrize a string in terminfo db; this should mean that parameter
count should't take into account "%d", "%s" and "%[1-9]" codes.

On top of that, cp[1] usage in the original code sounds like a mistake.

Please consider that I'm not a C programmer and this is only a "work for
me" proposal (even if I tested it with typical tput usage and the resulting
code seems closer to NetBSD's implementation).

Just let me know your thoughts.

[snip]
--- /usr/src/usr.bin/tput/tput.c.orig   Fri Jan 16 07:40:13 2015
+++ /usr/src/usr.bin/tput/tput.cSat Nov 21 12:26:13 2015
@@ -190,47 +190,30 @@
 process(char *cap, char *str, char **argv)
 {
char *cp, *s, *nargv[9];
-   int arg_need, popcount, i;
+   int arg_need, i;
 
/* Count how many values we need for this capability. */
-   for (cp = str, arg_need = popcount = 0; *cp != '\0'; cp++) {
+   for (cp = str, arg_need = 0; *cp != '\0'; cp++) {
if (*cp == '%') {
switch (*++cp) {
case '%':
cp++;
break;
case 'i':
-   if (popcount < 2)
-   popcount = 2;
+   if (arg_need < 2)
+   arg_need = 2;
break;
case 'p':
cp++;
-   if (isdigit((unsigned char)cp[1]) &&
-   popcount < cp[1] - '0')
-   popcount = cp[1] - '0';
+   if (isdigit((unsigned char)cp[0]) &&
+   arg_need < cp[0] - '0')
+   arg_need = cp[0] - '0';
break;
-   case 'd':
-   case 's':
-   case '0':
-   case '1':
-   case '2':
-   case '3':
-   case '4':
-   case '5':
-   case '6':
-   case '7':
-   case '8':
-   case '9':
-   case '.':
-   case '+':
-   arg_need++;
-   break;
default:
break;
}
}
}
-   arg_need = MAXIMUM(arg_need, popcount);
if (arg_need > 9)
errx(2, "too many arguments (%d) for capability `%s'",
arg_need, cap);
[snip]

[1] http://marc.info/?l=openbsd-misc=144800414402238=2 

-- 
Alessandro DE LAURENZIS
[mailto:just22@gmail.com]
LinkedIn: http://it.linkedin.com/in/delaurenzis

Re: calendar: add getpw promise

[Sebastien Marie]

On Sat, Nov 21, 2015 at 01:23:09PM +0100, Pascal Stumpf wrote:
> > if (doall) {
> > -   if (pledge("stdio rpath tmppath fattr id proc exec", NULL)
> > +   if (pledge("stdio rpath tmppath fattr getpw id proc exec", NULL)
> > == -1)
> > err(1, "pledge");
> > } else {
> > -   if (pledge("stdio rpath proc exec", NULL) == -1)
> > +   if (pledge("stdio rpath getpw proc exec", NULL) == -1)
> 
> The getpwent() call is below if (doall), so I think this is not needed.
> 
> > err(1, "pledge");
> > }
> >  

yes, you are right. matthieu@ said the same... my bad.

-- 
Sebastien Marie


Index: calendar.c
===
RCS file: /cvs/src/usr.bin/calendar/calendar.c,v
retrieving revision 1.33
diff -u -p -r1.33 calendar.c
--- calendar.c  16 Nov 2015 17:49:35 -  1.33
+++ calendar.c  21 Nov 2015 12:35:46 -
@@ -119,7 +119,7 @@ main(int argc, char *argv[])
usage();
 
if (doall) {
-   if (pledge("stdio rpath tmppath fattr id proc exec", NULL)
+   if (pledge("stdio rpath tmppath fattr getpw id proc exec", NULL)
== -1)
err(1, "pledge");
} else {

Re: Interesting if_get() case

[Martin Pieuchot]

On 20/11/15(Fri) 16:50, Alexandr Nedvedicky wrote:
> Hello,
> 
> thanks for detailed explanation.

Your welcome.

> 
> > +   else {
> > +   struct ifnet *destifp;
> > +
> > +   destifp = if_get(rt->rt_ifidx);
> > +   if (destifp != NULL)
> > +   destmtu = destifp->if_mtu;
> > +   if_put(destifp);
> > +   }
> 
> your code potentially leaves destmtu set to 0 in case we deal with invalid
> ipforward_rt. I wonder how icmp_error() we are going to call further below
> (at line 1544 in the old code) is going to deal with it.  May be we
> should just give up on sending ICMP_UNREACH message in this case.
> find my small improvement to your patch further below.

Note that my change should only matter for IPSEC.  This whole MTU mess is
scary. I can't even tell if it makes sense to send an ICMP if the ifp
disappeared.  The use of if_get(9) makes it clear that the MTU handling
is insane.

So I'm not opposed to your change but I think you should either commit
it separately or keep it inside the "if (ipforward_rt.ro_rt) {" block,
because it's a behavior change.

> Index: ip_input.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.261
> diff -u -p -r1.261 ip_input.c
> --- ip_input.c  14 Nov 2015 15:40:40 -  1.261
> +++ ip_input.c  20 Nov 2015 15:48:47 -
> @@ -1516,11 +1516,24 @@ ip_forward(struct mbuf *m, struct ifnet
> 
> if (rt->rt_rmx.rmx_mtu)
> destmtu = rt->rt_rmx.rmx_mtu;
> -   else
> -   destmtu = ipforward_rt.ro_rt->rt_ifp->if_mtu;
> +   else {
> +   struct ifnet *destifp;
> +
> +   destifp = if_get(rt->rt_ifidx);
> +   if (destifp != NULL)
> +   destmtu = destifp->if_mtu;
> +   if_put(destifp);
> +   }
> }
>  #endif /*IPSEC*/
> ipstat.ips_cantfrag++;
> +
> +   /*
> +* route to destniation no longer exists, we should revert 
> code
> +* back to host unreachable.
> +*/
> +   if (destmtu == 0)
> +   code = ICMP_UNREACH_HOST;
> break;
> 
> case EACCES:
> 
> 
>