[patch] vi: fix "file modified more recently than this copy ..." error

2015-06-09 Thread patrick keshishian
Hi,

Noticed a regression with vi and recent changes to timespec
data types.

To reproduce, run vi without a file name to edit. Try save buffer
via ":w" and you'll be presented by following message:

file modified more recently than this this copy; use ! to override

Patch below fixes this.

Hope this is acceptable.

Thanks,
--patrick

Index: common/exf.c
===
RCS file: /cvs/obsd/src/usr.bin/vi/common/exf.c,v
retrieving revision 1.36
diff -u -p -u -p -r1.36 exf.c
--- common/exf.c24 Apr 2015 21:48:31 -  1.36
+++ common/exf.c10 Jun 2015 03:55:58 -
@@ -211,7 +211,10 @@ file_init(SCR *sp, FREF *frp, char *rcv_
if (!LF_ISSET(FS_OPENERR))
F_SET(frp, FR_NEWFILE);
 
-   (void)clock_gettime(CLOCK_REALTIME, &ep->mtim);
+   if (stat(tname, &sb))
+   (void)clock_gettime(CLOCK_REALTIME, &ep->mtim);
+   else
+   ep->mtim = sb.st_mtim;
} else {
/*
 * XXX



Re: RIP hzto(9) 3/4: the implementation

2015-06-09 Thread David Gwynne
ok.

> On 10 Jun 2015, at 07:42, Mike Belopuhov  wrote:
> 
> OK?
> 
> diff --git sys/kern/kern_clock.c sys/kern/kern_clock.c
> index 279804c..e35f2f4 100644
> --- sys/kern/kern_clock.c
> +++ sys/kern/kern_clock.c
> @@ -200,63 +200,10 @@ hardclock(struct clockframe *frame)
>   if (timeout_hardclock_update())
>   softintr_schedule(softclock_si);
> }
> 
> /*
> - * Compute number of hz until specified time.  Used to
> - * compute the second argument to timeout_add() from an absolute time.
> - */
> -int
> -hzto(const struct timeval *tv)
> -{
> - struct timeval now;
> - unsigned long nticks;
> - long sec, usec;
> -
> - /*
> -  * If the number of usecs in the whole seconds part of the time
> -  * difference fits in a long, then the total number of usecs will
> -  * fit in an unsigned long.  Compute the total and convert it to
> -  * ticks, rounding up and adding 1 to allow for the current tick
> -  * to expire.  Rounding also depends on unsigned long arithmetic
> -  * to avoid overflow.
> -  *
> -  * Otherwise, if the number of ticks in the whole seconds part of
> -  * the time difference fits in a long, then convert the parts to
> -  * ticks separately and add, using similar rounding methods and
> -  * overflow avoidance.  This method would work in the previous
> -  * case but it is slightly slower and assumes that hz is integral.
> -  *
> -  * Otherwise, round the time difference down to the maximum
> -  * representable value.
> -  *
> -  * If ints have 32 bits, then the maximum value for any timeout in
> -  * 10ms ticks is 248 days.
> -  */
> - getmicrotime(&now);
> - sec = tv->tv_sec - now.tv_sec;
> - usec = tv->tv_usec - now.tv_usec;
> - if (usec < 0) {
> - sec--;
> - usec += 100;
> - }
> - if (sec < 0 || (sec == 0 && usec <= 0)) {
> - nticks = 0;
> - } else if (sec <= LONG_MAX / 100)
> - nticks = (sec * 100 + (unsigned long)usec + (tick - 1))
> - / tick + 1;
> - else if (sec <= LONG_MAX / hz)
> - nticks = sec * hz
> - + ((unsigned long)usec + (tick - 1)) / tick + 1;
> - else
> - nticks = LONG_MAX;
> - if (nticks > INT_MAX)
> - nticks = INT_MAX;
> - return ((int)nticks);
> -}
> -
> -/*
>  * Compute number of hz in the specified amount of time.
>  */
> int
> tvtohz(const struct timeval *tv)
> {
> diff --git sys/sys/systm.h sys/sys/systm.h
> index 26f1f57..98bd959 100644
> --- sys/sys/systm.h
> +++ sys/sys/systm.h
> @@ -220,11 +220,10 @@ voidarc4random_buf(void *, size_t)
> u_int32_t arc4random(void);
> u_int32_t arc4random_uniform(u_int32_t);
> 
> struct timeval;
> struct timespec;
> -int  hzto(const struct timeval *);
> int   tvtohz(const struct timeval *);
> int   tstohz(const struct timespec *);
> void  realitexpire(void *);
> 
> struct clockframe;
> 



Re: RIP hzto(9) 4/4: manual pages

2015-06-09 Thread David Gwynne

> On 10 Jun 2015, at 07:43, Mike Belopuhov  wrote:
> 
> OK?

yep.

> 
> diff --git share/man/man9/Makefile share/man/man9/Makefile
> index d145186..c63132f 100644
> --- share/man/man9/Makefile
> +++ share/man/man9/Makefile
> @@ -12,11 +12,11 @@ MAN=  aml_evalnode.9 atomic_add_int.9 
> atomic_cas_uint.9 \
>   copy.9 config_attach.9 crypto.9 delay.9 \
>   disk.9 disklabel.9 dma_alloc.9 dohooks.9 dopowerhooks.9 \
>   domountroothooks.9 dostartuphooks.9 \
>   evcount.9 extent.9 fb_setup.9 file.9 fork1.9 \
>   getdevvp.9 getnewvnode.9 hash.9 hashinit.9 \
> - hardclock.9 hook_establish.9 hz.9 hzto.9 idgen32.9 \
> + hardclock.9 hook_establish.9 hz.9 idgen32.9 \
>   ieee80211.9 ieee80211_crypto.9 ieee80211_input.9 ieee80211_ioctl.9 \
>   ieee80211_node.9 ieee80211_output.9 ieee80211_proto.9 \
>   ieee80211_radiotap.9 \
>   if_rxr_init.9 iic.9 intro.9 inittodr.9 \
>   kern.9 km_alloc.9 knote.9 kthread.9 ktrace.9 \
> diff --git share/man/man9/hzto.9 share/man/man9/hzto.9
> deleted file mode 100644
> index 907a59a..000
> --- share/man/man9/hzto.9
> +++ /dev/null
> @@ -1,52 +0,0 @@
> -.\"  $OpenBSD: hzto.9,v 1.8 2013/06/04 19:27:07 schwarze Exp $
> -.\"
> -.\" Copyright (c) 1999 Marc Espie
> -.\" All rights reserved.
> -.\"
> -.\" Redistribution and use in source and binary forms, with or without
> -.\" modification, are permitted provided that the following conditions
> -.\" are met:
> -.\" 1. Redistributions of source code must retain the above copyright
> -.\"notice, this list of conditions and the following disclaimer.
> -.\" 2. Redistributions in binary form must reproduce the above copyright
> -.\"notice, this list of conditions and the following disclaimer in the
> -.\"documentation and/or other materials provided with the distribution.
> -.\"
> -.\" THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
> -.\" IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> -.\" OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
> -.\" IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> -.\" INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> -.\" NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> -.\" DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> -.\" THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> -.\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> -.\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> -.\"
> -.Dd $Mdocdate: June 4 2013 $
> -.Dt HZTO 9
> -.Os
> -.Sh NAME
> -.Nm hzto
> -.Nd translate absolute time to timeout delay
> -.Sh SYNOPSIS
> -.In sys/types.h
> -.In sys/time.h
> -.In sys/systm.h
> -.Ft int
> -.Fn hzto "const struct timeval *tv"
> -.Sh DESCRIPTION
> -The
> -.Fn hzto
> -function computes the number of
> -.Va hz
> -until the specified time occurs.
> -This is mainly used to translate a timeval into a suitable argument for
> -.Xr timeout 9 .
> -.Sh CODE REFERENCES
> -This function is implemented in the file
> -.Pa sys/kern/kern_clock.c .
> -.Sh SEE ALSO
> -.Xr hz 9 ,
> -.Xr timeout 9 ,
> -.Xr tvtohz 9
> diff --git share/man/man9/timeout.9 share/man/man9/timeout.9
> index e741921..e716e36 100644
> --- share/man/man9/timeout.9
> +++ share/man/man9/timeout.9
> @@ -233,9 +233,8 @@ currently queued.
> .Sh CODE REFERENCES
> These functions are implemented in the file
> .Pa sys/kern/kern_timeout.c .
> .Sh SEE ALSO
> .Xr hz 9 ,
> -.Xr hzto 9 ,
> .Xr splclock 9 ,
> .Xr tsleep 9 ,
> .Xr tvtohz 9
> diff --git share/man/man9/tvtohz.9 share/man/man9/tvtohz.9
> index bcd2c8e..53e49a3 100644
> --- share/man/man9/tvtohz.9
> +++ share/man/man9/tvtohz.9
> @@ -53,7 +53,6 @@ into a suitable argument for
> .Sh CODE REFERENCES
> These functions are implemented in the file
> .Pa sys/kern/kern_clock.c .
> .Sh SEE ALSO
> .Xr hz 9 ,
> -.Xr hzto 9 ,
> .Xr timeout 9
> 




Re: RIP hzto(9) 2/4: NFS

2015-06-09 Thread David Gwynne
ok.

> On 10 Jun 2015, at 07:41, Mike Belopuhov  wrote:
> 
> OK?
> 
> diff --git sys/nfs/nfs_socket.c sys/nfs/nfs_socket.c
> index 9edd615..a4a279f 100644
> --- sys/nfs/nfs_socket.c
> +++ sys/nfs/nfs_socket.c
> @@ -1003,13 +1003,13 @@ tryagain:
>   error = fxdr_unsigned(int, *tl);
>   if ((nmp->nm_flag & NFSMNT_NFSV3) &&
>   error == NFSERR_TRYLATER) {
>   m_freem(info.nmi_mrep);
>   error = 0;
> - tv.tv_sec = time_second + trylater_delay;
> + tv.tv_sec = trylater_delay;
>   tv.tv_usec = 0;
> - tsleep(&tv, PSOCK, "nfsretry", hzto(&tv));
> + tsleep(&tv, PSOCK, "nfsretry", tvtohz(&tv));
>   trylater_delay *= NFS_TIMEOUTMUL;
>   if (trylater_delay > NFS_MAXTIMEO)
>   trylater_delay = NFS_MAXTIMEO;
> 
>   goto tryagain;
> 



Re: RIP hzto(9) 1/4: IPsec

2015-06-09 Thread David Gwynne
ok.

> On 10 Jun 2015, at 07:41, Mike Belopuhov  wrote:
> 
> OK?
> 
> diff --git sys/net/if_bridge.c sys/net/if_bridge.c
> index 637dea8..ce8d0d7 100644
> --- sys/net/if_bridge.c
> +++ sys/net/if_bridge.c
> @@ -2181,11 +2181,10 @@ int
> bridge_ipsec(struct bridge_softc *sc, struct ifnet *ifp,
> struct ether_header *eh, int hassnap, struct llc *llc,
> int dir, int af, int hlen, struct mbuf *m)
> {
>   union sockaddr_union dst;
> - struct timeval tv;
>   struct tdb *tdb;
>   u_int32_t spi;
>   u_int16_t cpi;
>   int error, off, s;
>   u_int8_t proto = 0;
> @@ -2277,37 +2276,16 @@ bridge_ipsec(struct bridge_softc *sc, struct ifnet 
> *ifp,
>   tdb = gettdb(ifp->if_rdomain, spi, &dst, proto);
>   if (tdb != NULL && (tdb->tdb_flags & TDBF_INVALID) == 0 &&
>   tdb->tdb_xform != NULL) {
>   if (tdb->tdb_first_use == 0) {
>   tdb->tdb_first_use = time_second;
> -
> - tv.tv_usec = 0;
> -
> - /* Check for wrap-around. */
> - if (tdb->tdb_exp_first_use + tdb->tdb_first_use
> - < tdb->tdb_first_use)
> - tv.tv_sec = ((unsigned long)-1) / 2;
> - else
> - tv.tv_sec = tdb->tdb_exp_first_use +
> - tdb->tdb_first_use;
> -
>   if (tdb->tdb_flags & TDBF_FIRSTUSE)
> - timeout_add(&tdb->tdb_first_tmo,
> - hzto(&tv));
> -
> - /* Check for wrap-around. */
> - if (tdb->tdb_first_use +
> - tdb->tdb_soft_first_use
> - < tdb->tdb_first_use)
> - tv.tv_sec = ((unsigned long)-1) / 2;
> - else
> - tv.tv_sec = tdb->tdb_first_use +
> - tdb->tdb_soft_first_use;
> -
> + timeout_add_sec(&tdb->tdb_first_tmo,
> + tdb->tdb_exp_first_use);
>   if (tdb->tdb_flags & TDBF_SOFT_FIRSTUSE)
> - timeout_add(&tdb->tdb_sfirst_tmo,
> - hzto(&tv));
> + timeout_add_sec(&tdb->tdb_sfirst_tmo,
> + tdb->tdb_soft_first_use);
>   }
> 
>   (*(tdb->tdb_xform->xf_input))(m, tdb, hlen, off);
>   splx(s);
>   return (1);
> diff --git sys/net/pfkeyv2_convert.c sys/net/pfkeyv2_convert.c
> index 9fa4920..6657a51 100644
> --- sys/net/pfkeyv2_convert.c
> +++ sys/net/pfkeyv2_convert.c
> @@ -277,17 +277,13 @@ export_sa(void **p, struct tdb *tdb)
>  * Initialize expirations and counters based on lifetime payload.
>  */
> void
> import_lifetime(struct tdb *tdb, struct sadb_lifetime *sadb_lifetime, int 
> type)
> {
> - struct timeval tv;
> -
>   if (!sadb_lifetime)
>   return;
> 
> - getmicrotime(&tv);
> -
>   switch (type) {
>   case PFKEYV2_LIFETIME_HARD:
>   if ((tdb->tdb_exp_allocations =
>   sadb_lifetime->sadb_lifetime_allocations) != 0)
>   tdb->tdb_flags |= TDBF_ALLOCATIONS;
> @@ -301,15 +297,12 @@ import_lifetime(struct tdb *tdb, struct sadb_lifetime 
> *sadb_lifetime, int type)
>   tdb->tdb_flags &= ~TDBF_BYTES;
> 
>   if ((tdb->tdb_exp_timeout =
>   sadb_lifetime->sadb_lifetime_addtime) != 0) {
>   tdb->tdb_flags |= TDBF_TIMER;
> - if (tv.tv_sec + tdb->tdb_exp_timeout < tv.tv_sec)
> - tv.tv_sec = ((unsigned long) -1) / 2; /* XXX */
> - else
> - tv.tv_sec += tdb->tdb_exp_timeout;
> - timeout_add(&tdb->tdb_timer_tmo, hzto(&tv));
> + timeout_add_sec(&tdb->tdb_timer_tmo,
> + tdb->tdb_exp_timeout);
>   } else
>   tdb->tdb_flags &= ~TDBF_TIMER;
> 
>   if ((tdb->tdb_exp_first_use =
>   sadb_lifetime->sadb_lifetime_usetime) != 0)
> @@ -332,15 +325,12 @@ import_lifetime(struct tdb *tdb, struct sadb_lifetime 
> *sadb_lifetime, int type)
>   tdb->tdb_flags &= ~TDBF_SOFT_BYTES;
> 
>   if ((tdb->tdb_soft_timeout =
>   sadb_lifetime->sadb_lifetime_addtime) != 0) {
>   tdb->tdb_flags |= TDBF_SOFT_TIMER;
> - if (tv.tv_sec + tdb->tdb_soft_timeout < tv.tv_sec)
> - 

Re: clean the tree from superfluous "case '?': /* FALLTHROUGH */"

2015-06-09 Thread Benjamin Baier
On Tue, 9 Jun 2015 14:01:01 -0700
patrick keshishian  wrote:

> Hi,
> 
> On Tue, Jun 09, 2015 at 07:46:20PM +0200, Benjamin Baier wrote:
> > Hello tech@
> > 
> > delete "case '?': /* FALLTHROUGH */" where it is already handled 
> > by "default: usage();".
> 
> Not quite sure it is correct to remove the '?' case from
> npppd.c, since there is no 'default' case.
Best to leave it out. Yes you are right this alters behaviour (usage not 
printed) when invalid option arguments are passed. Had it on the list to
revisit later and add a default case.

> Also, if the idea is to remove cases which simply fall through
> 'default', these few (10) have 'h' cases that do same (judging
> from your diff):
> 
> > RCS file: /cvs/src/games/arithmetic/arithmetic.c,v
> > RCS file: /cvs/src/games/banner/banner.c,v
> > RCS file: /cvs/src/games/fish/fish.c,v
> > RCS file: /cvs/src/games/grdc/grdc.c,v
> > RCS file: /cvs/src/games/hangman/main.c,v
> > RCS file: /cvs/src/games/morse/morse.c,v
> > RCS file: /cvs/src/games/number/number.c,v
> > RCS file: /cvs/src/games/ppt/ppt.c,v
> > RCS file: /cvs/src/games/snake/snake.c,v
> > RCS file: /cvs/src/games/worms/worms.c,v
I was focusing on case '?', and because 'h' is a valid option argument i
decided not to delete the case 'h', for now.
But i'm happy to add them to the patch, if requested.
 
> --patrick
> 
> 
> ...
> > Index: games/arithmetic/arithmetic.c
> > ===
> > RCS file: /cvs/src/games/arithmetic/arithmetic.c,v
> > retrieving revision 1.18
> > diff -u -p -r1.18 arithmetic.c
> > --- games/arithmetic/arithmetic.c   29 Aug 2013 20:22:09 -  1.18
> > +++ games/arithmetic/arithmetic.c   9 Jun 2015 17:28:09 -
> > @@ -116,7 +116,6 @@ main(int argc, char *argv[])
> > if ((rangemax = atoi(optarg)) <= 0)
> > errx(1, "invalid range.");
> > break;
> > -   case '?':
> > case 'h':
> > default:
> > usage();
> > Index: games/banner/banner.c
> > ===
> > RCS file: /cvs/src/games/banner/banner.c,v
> > retrieving revision 1.17
> > diff -u -p -r1.17 banner.c
> > --- games/banner/banner.c   10 Feb 2015 13:50:58 -  1.17
> > +++ games/banner/banner.c   9 Jun 2015 17:28:09 -
> > @@ -1030,7 +1030,7 @@ main(int argc, char *argv[])
> > if (width <= 0 || width > DWIDTH)
> > errx(1, "illegal argument for -w option");
> > break;
> > -   case '?': case 'h':
> > +   case 'h':
> > default:
> > (void)fprintf(stderr,
> > "usage: banner [-w width] message ...\n");
> ...
> > ===
> > RCS file: /cvs/src/games/fish/fish.c,v
> > retrieving revision 1.17
> > diff -u -p -r1.17 fish.c
> > --- games/fish/fish.c   18 Feb 2015 23:20:45 -  1.17
> > +++ games/fish/fish.c   9 Jun 2015 17:28:09 -
> > @@ -92,7 +92,6 @@ main(int argc, char *argv[])
> > case 'p':
> > promode = 1;
> > break;
> > -   case '?':
> > case 'h':
> > default:
> > usage();
> > }
> ...
> > Index: games/grdc/grdc.c
> > ===
> > RCS file: /cvs/src/games/grdc/grdc.c,v
> > retrieving revision 1.19
> > diff -u -p -r1.19 grdc.c
> > --- games/grdc/grdc.c   19 Nov 2014 03:27:45 -  1.19
> > +++ games/grdc/grdc.c   9 Jun 2015 17:28:09 -
> > @@ -77,7 +77,6 @@ main(int argc, char *argv[])
> > scrol = 1;
> > break;
> > case 'h':
> > -   case '?':
> > default:
> > usage();
> > }
> > Index: games/hangman/main.c
> > ===
> > RCS file: /cvs/src/games/hangman/main.c,v
> > retrieving revision 1.12
> > diff -u -p -r1.12 main.c
> > --- games/hangman/main.c7 Feb 2015 01:37:30 -   1.12
> > +++ games/hangman/main.c9 Jun 2015 17:28:09 -
> > @@ -56,7 +56,6 @@ main(int argc, char *argv[])
> > Dict_name = _PATH_KSYMS;
> > break;
> > case 'h':
> > -   case '?':
> > default:
> > usage();
> > }
> > Index: games/morse/morse.c
> > ===
> > RCS file: /cvs/src/games/morse/morse.c,v
> > retrieving revision 1.15
> > diff -u -p -r1.15 morse.c
> > --- games/morse/morse.c 27 Nov 2013 13:32:02 -  1.15
> > +++ games/morse/morse.c 9 Jun 2015 17:28:09 -
> > @@ -120,7 +120,7 @@ main(int argc, char *argv[])
> > case 's':
> > sfl

RIP hzto(9) 4/4: manual pages

2015-06-09 Thread Mike Belopuhov
OK?

diff --git share/man/man9/Makefile share/man/man9/Makefile
index d145186..c63132f 100644
--- share/man/man9/Makefile
+++ share/man/man9/Makefile
@@ -12,11 +12,11 @@ MAN=aml_evalnode.9 atomic_add_int.9 
atomic_cas_uint.9 \
copy.9 config_attach.9 crypto.9 delay.9 \
disk.9 disklabel.9 dma_alloc.9 dohooks.9 dopowerhooks.9 \
domountroothooks.9 dostartuphooks.9 \
evcount.9 extent.9 fb_setup.9 file.9 fork1.9 \
getdevvp.9 getnewvnode.9 hash.9 hashinit.9 \
-   hardclock.9 hook_establish.9 hz.9 hzto.9 idgen32.9 \
+   hardclock.9 hook_establish.9 hz.9 idgen32.9 \
ieee80211.9 ieee80211_crypto.9 ieee80211_input.9 ieee80211_ioctl.9 \
ieee80211_node.9 ieee80211_output.9 ieee80211_proto.9 \
ieee80211_radiotap.9 \
if_rxr_init.9 iic.9 intro.9 inittodr.9 \
kern.9 km_alloc.9 knote.9 kthread.9 ktrace.9 \
diff --git share/man/man9/hzto.9 share/man/man9/hzto.9
deleted file mode 100644
index 907a59a..000
--- share/man/man9/hzto.9
+++ /dev/null
@@ -1,52 +0,0 @@
-.\"$OpenBSD: hzto.9,v 1.8 2013/06/04 19:27:07 schwarze Exp $
-.\"
-.\" Copyright (c) 1999 Marc Espie
-.\" All rights reserved.
-.\"
-.\" Redistribution and use in source and binary forms, with or without
-.\" modification, are permitted provided that the following conditions
-.\" are met:
-.\" 1. Redistributions of source code must retain the above copyright
-.\"notice, this list of conditions and the following disclaimer.
-.\" 2. Redistributions in binary form must reproduce the above copyright
-.\"notice, this list of conditions and the following disclaimer in the
-.\"documentation and/or other materials provided with the distribution.
-.\"
-.\" THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
-.\" IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
-.\" OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
-.\" IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
-.\" INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
-.\" NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-.\" DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-.\" THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-.\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
-.\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-.\"
-.Dd $Mdocdate: June 4 2013 $
-.Dt HZTO 9
-.Os
-.Sh NAME
-.Nm hzto
-.Nd translate absolute time to timeout delay
-.Sh SYNOPSIS
-.In sys/types.h
-.In sys/time.h
-.In sys/systm.h
-.Ft int
-.Fn hzto "const struct timeval *tv"
-.Sh DESCRIPTION
-The
-.Fn hzto
-function computes the number of
-.Va hz
-until the specified time occurs.
-This is mainly used to translate a timeval into a suitable argument for
-.Xr timeout 9 .
-.Sh CODE REFERENCES
-This function is implemented in the file
-.Pa sys/kern/kern_clock.c .
-.Sh SEE ALSO
-.Xr hz 9 ,
-.Xr timeout 9 ,
-.Xr tvtohz 9
diff --git share/man/man9/timeout.9 share/man/man9/timeout.9
index e741921..e716e36 100644
--- share/man/man9/timeout.9
+++ share/man/man9/timeout.9
@@ -233,9 +233,8 @@ currently queued.
 .Sh CODE REFERENCES
 These functions are implemented in the file
 .Pa sys/kern/kern_timeout.c .
 .Sh SEE ALSO
 .Xr hz 9 ,
-.Xr hzto 9 ,
 .Xr splclock 9 ,
 .Xr tsleep 9 ,
 .Xr tvtohz 9
diff --git share/man/man9/tvtohz.9 share/man/man9/tvtohz.9
index bcd2c8e..53e49a3 100644
--- share/man/man9/tvtohz.9
+++ share/man/man9/tvtohz.9
@@ -53,7 +53,6 @@ into a suitable argument for
 .Sh CODE REFERENCES
 These functions are implemented in the file
 .Pa sys/kern/kern_clock.c .
 .Sh SEE ALSO
 .Xr hz 9 ,
-.Xr hzto 9 ,
 .Xr timeout 9



RIP hzto(9) 3/4: the implementation

2015-06-09 Thread Mike Belopuhov
OK?

diff --git sys/kern/kern_clock.c sys/kern/kern_clock.c
index 279804c..e35f2f4 100644
--- sys/kern/kern_clock.c
+++ sys/kern/kern_clock.c
@@ -200,63 +200,10 @@ hardclock(struct clockframe *frame)
if (timeout_hardclock_update())
softintr_schedule(softclock_si);
 }
 
 /*
- * Compute number of hz until specified time.  Used to
- * compute the second argument to timeout_add() from an absolute time.
- */
-int
-hzto(const struct timeval *tv)
-{
-   struct timeval now;
-   unsigned long nticks;
-   long sec, usec;
-
-   /*
-* If the number of usecs in the whole seconds part of the time
-* difference fits in a long, then the total number of usecs will
-* fit in an unsigned long.  Compute the total and convert it to
-* ticks, rounding up and adding 1 to allow for the current tick
-* to expire.  Rounding also depends on unsigned long arithmetic
-* to avoid overflow.
-*
-* Otherwise, if the number of ticks in the whole seconds part of
-* the time difference fits in a long, then convert the parts to
-* ticks separately and add, using similar rounding methods and
-* overflow avoidance.  This method would work in the previous
-* case but it is slightly slower and assumes that hz is integral.
-*
-* Otherwise, round the time difference down to the maximum
-* representable value.
-*
-* If ints have 32 bits, then the maximum value for any timeout in
-* 10ms ticks is 248 days.
-*/
-   getmicrotime(&now);
-   sec = tv->tv_sec - now.tv_sec;
-   usec = tv->tv_usec - now.tv_usec;
-   if (usec < 0) {
-   sec--;
-   usec += 100;
-   }
-   if (sec < 0 || (sec == 0 && usec <= 0)) {
-   nticks = 0;
-   } else if (sec <= LONG_MAX / 100)
-   nticks = (sec * 100 + (unsigned long)usec + (tick - 1))
-   / tick + 1;
-   else if (sec <= LONG_MAX / hz)
-   nticks = sec * hz
-   + ((unsigned long)usec + (tick - 1)) / tick + 1;
-   else
-   nticks = LONG_MAX;
-   if (nticks > INT_MAX)
-   nticks = INT_MAX;
-   return ((int)nticks);
-}
-
-/*
  * Compute number of hz in the specified amount of time.
  */
 int
 tvtohz(const struct timeval *tv)
 {
diff --git sys/sys/systm.h sys/sys/systm.h
index 26f1f57..98bd959 100644
--- sys/sys/systm.h
+++ sys/sys/systm.h
@@ -220,11 +220,10 @@ void  arc4random_buf(void *, size_t)
 u_int32_t arc4random(void);
 u_int32_t arc4random_uniform(u_int32_t);
 
 struct timeval;
 struct timespec;
-inthzto(const struct timeval *);
 inttvtohz(const struct timeval *);
 inttstohz(const struct timespec *);
 void   realitexpire(void *);
 
 struct clockframe;



RIP hzto(9) 2/4: NFS

2015-06-09 Thread Mike Belopuhov
OK?

diff --git sys/nfs/nfs_socket.c sys/nfs/nfs_socket.c
index 9edd615..a4a279f 100644
--- sys/nfs/nfs_socket.c
+++ sys/nfs/nfs_socket.c
@@ -1003,13 +1003,13 @@ tryagain:
error = fxdr_unsigned(int, *tl);
if ((nmp->nm_flag & NFSMNT_NFSV3) &&
error == NFSERR_TRYLATER) {
m_freem(info.nmi_mrep);
error = 0;
-   tv.tv_sec = time_second + trylater_delay;
+   tv.tv_sec = trylater_delay;
tv.tv_usec = 0;
-   tsleep(&tv, PSOCK, "nfsretry", hzto(&tv));
+   tsleep(&tv, PSOCK, "nfsretry", tvtohz(&tv));
trylater_delay *= NFS_TIMEOUTMUL;
if (trylater_delay > NFS_MAXTIMEO)
trylater_delay = NFS_MAXTIMEO;
 
goto tryagain;



RIP hzto(9) 1/4: IPsec

2015-06-09 Thread Mike Belopuhov
OK?

diff --git sys/net/if_bridge.c sys/net/if_bridge.c
index 637dea8..ce8d0d7 100644
--- sys/net/if_bridge.c
+++ sys/net/if_bridge.c
@@ -2181,11 +2181,10 @@ int
 bridge_ipsec(struct bridge_softc *sc, struct ifnet *ifp,
 struct ether_header *eh, int hassnap, struct llc *llc,
 int dir, int af, int hlen, struct mbuf *m)
 {
union sockaddr_union dst;
-   struct timeval tv;
struct tdb *tdb;
u_int32_t spi;
u_int16_t cpi;
int error, off, s;
u_int8_t proto = 0;
@@ -2277,37 +2276,16 @@ bridge_ipsec(struct bridge_softc *sc, struct ifnet *ifp,
tdb = gettdb(ifp->if_rdomain, spi, &dst, proto);
if (tdb != NULL && (tdb->tdb_flags & TDBF_INVALID) == 0 &&
tdb->tdb_xform != NULL) {
if (tdb->tdb_first_use == 0) {
tdb->tdb_first_use = time_second;
-
-   tv.tv_usec = 0;
-
-   /* Check for wrap-around. */
-   if (tdb->tdb_exp_first_use + tdb->tdb_first_use
-   < tdb->tdb_first_use)
-   tv.tv_sec = ((unsigned long)-1) / 2;
-   else
-   tv.tv_sec = tdb->tdb_exp_first_use +
-   tdb->tdb_first_use;
-
if (tdb->tdb_flags & TDBF_FIRSTUSE)
-   timeout_add(&tdb->tdb_first_tmo,
-   hzto(&tv));
-
-   /* Check for wrap-around. */
-   if (tdb->tdb_first_use +
-   tdb->tdb_soft_first_use
-   < tdb->tdb_first_use)
-   tv.tv_sec = ((unsigned long)-1) / 2;
-   else
-   tv.tv_sec = tdb->tdb_first_use +
-   tdb->tdb_soft_first_use;
-
+   timeout_add_sec(&tdb->tdb_first_tmo,
+   tdb->tdb_exp_first_use);
if (tdb->tdb_flags & TDBF_SOFT_FIRSTUSE)
-   timeout_add(&tdb->tdb_sfirst_tmo,
-   hzto(&tv));
+   timeout_add_sec(&tdb->tdb_sfirst_tmo,
+   tdb->tdb_soft_first_use);
}
 
(*(tdb->tdb_xform->xf_input))(m, tdb, hlen, off);
splx(s);
return (1);
diff --git sys/net/pfkeyv2_convert.c sys/net/pfkeyv2_convert.c
index 9fa4920..6657a51 100644
--- sys/net/pfkeyv2_convert.c
+++ sys/net/pfkeyv2_convert.c
@@ -277,17 +277,13 @@ export_sa(void **p, struct tdb *tdb)
  * Initialize expirations and counters based on lifetime payload.
  */
 void
 import_lifetime(struct tdb *tdb, struct sadb_lifetime *sadb_lifetime, int type)
 {
-   struct timeval tv;
-
if (!sadb_lifetime)
return;
 
-   getmicrotime(&tv);
-
switch (type) {
case PFKEYV2_LIFETIME_HARD:
if ((tdb->tdb_exp_allocations =
sadb_lifetime->sadb_lifetime_allocations) != 0)
tdb->tdb_flags |= TDBF_ALLOCATIONS;
@@ -301,15 +297,12 @@ import_lifetime(struct tdb *tdb, struct sadb_lifetime 
*sadb_lifetime, int type)
tdb->tdb_flags &= ~TDBF_BYTES;
 
if ((tdb->tdb_exp_timeout =
sadb_lifetime->sadb_lifetime_addtime) != 0) {
tdb->tdb_flags |= TDBF_TIMER;
-   if (tv.tv_sec + tdb->tdb_exp_timeout < tv.tv_sec)
-   tv.tv_sec = ((unsigned long) -1) / 2; /* XXX */
-   else
-   tv.tv_sec += tdb->tdb_exp_timeout;
-   timeout_add(&tdb->tdb_timer_tmo, hzto(&tv));
+   timeout_add_sec(&tdb->tdb_timer_tmo,
+   tdb->tdb_exp_timeout);
} else
tdb->tdb_flags &= ~TDBF_TIMER;
 
if ((tdb->tdb_exp_first_use =
sadb_lifetime->sadb_lifetime_usetime) != 0)
@@ -332,15 +325,12 @@ import_lifetime(struct tdb *tdb, struct sadb_lifetime 
*sadb_lifetime, int type)
tdb->tdb_flags &= ~TDBF_SOFT_BYTES;
 
if ((tdb->tdb_soft_timeout =
sadb_lifetime->sadb_lifetime_addtime) != 0) {
tdb->tdb_flags |= TDBF_SOFT_TIMER;
-   if (tv.tv_sec + tdb->tdb_soft_timeout < tv.tv_sec)
-   tv.tv_sec = ((unsigned long) -1) / 2; /* XXX */
-   else
-   tv.tv_

Re: clean the tree from superfluous "case '?': /* FALLTHROUGH */"

2015-06-09 Thread patrick keshishian
Hi,

On Tue, Jun 09, 2015 at 07:46:20PM +0200, Benjamin Baier wrote:
> Hello tech@
> 
> delete "case '?': /* FALLTHROUGH */" where it is already handled 
> by "default: usage();".

Not quite sure it is correct to remove the '?' case from
npppd.c, since there is no 'default' case.

Also, if the idea is to remove cases which simply fall through
'default', these few (10) have 'h' cases that do same (judging
from your diff):

> RCS file: /cvs/src/games/arithmetic/arithmetic.c,v
> RCS file: /cvs/src/games/banner/banner.c,v
> RCS file: /cvs/src/games/fish/fish.c,v
> RCS file: /cvs/src/games/grdc/grdc.c,v
> RCS file: /cvs/src/games/hangman/main.c,v
> RCS file: /cvs/src/games/morse/morse.c,v
> RCS file: /cvs/src/games/number/number.c,v
> RCS file: /cvs/src/games/ppt/ppt.c,v
> RCS file: /cvs/src/games/snake/snake.c,v
> RCS file: /cvs/src/games/worms/worms.c,v


--patrick


...
> Index: games/arithmetic/arithmetic.c
> ===
> RCS file: /cvs/src/games/arithmetic/arithmetic.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 arithmetic.c
> --- games/arithmetic/arithmetic.c 29 Aug 2013 20:22:09 -  1.18
> +++ games/arithmetic/arithmetic.c 9 Jun 2015 17:28:09 -
> @@ -116,7 +116,6 @@ main(int argc, char *argv[])
>   if ((rangemax = atoi(optarg)) <= 0)
>   errx(1, "invalid range.");
>   break;
> - case '?':
>   case 'h':
>   default:
>   usage();
> Index: games/banner/banner.c
> ===
> RCS file: /cvs/src/games/banner/banner.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 banner.c
> --- games/banner/banner.c 10 Feb 2015 13:50:58 -  1.17
> +++ games/banner/banner.c 9 Jun 2015 17:28:09 -
> @@ -1030,7 +1030,7 @@ main(int argc, char *argv[])
>   if (width <= 0 || width > DWIDTH)
>   errx(1, "illegal argument for -w option");
>   break;
> - case '?': case 'h':
> + case 'h':
>   default:
>   (void)fprintf(stderr,
>   "usage: banner [-w width] message ...\n");
...
> ===
> RCS file: /cvs/src/games/fish/fish.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 fish.c
> --- games/fish/fish.c 18 Feb 2015 23:20:45 -  1.17
> +++ games/fish/fish.c 9 Jun 2015 17:28:09 -
> @@ -92,7 +92,6 @@ main(int argc, char *argv[])
>   case 'p':
>   promode = 1;
>   break;
> - case '?':
>   case 'h':
>   default:
>   usage();
>   }
...
> Index: games/grdc/grdc.c
> ===
> RCS file: /cvs/src/games/grdc/grdc.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 grdc.c
> --- games/grdc/grdc.c 19 Nov 2014 03:27:45 -  1.19
> +++ games/grdc/grdc.c 9 Jun 2015 17:28:09 -
> @@ -77,7 +77,6 @@ main(int argc, char *argv[])
>   scrol = 1;
>   break;
>   case 'h':
> - case '?':
>   default:
>   usage();
>   }
> Index: games/hangman/main.c
> ===
> RCS file: /cvs/src/games/hangman/main.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 main.c
> --- games/hangman/main.c  7 Feb 2015 01:37:30 -   1.12
> +++ games/hangman/main.c  9 Jun 2015 17:28:09 -
> @@ -56,7 +56,6 @@ main(int argc, char *argv[])
>   Dict_name = _PATH_KSYMS;
>   break;
>   case 'h':
> - case '?':
>   default:
>   usage();
>   }
> Index: games/morse/morse.c
> ===
> RCS file: /cvs/src/games/morse/morse.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 morse.c
> --- games/morse/morse.c   27 Nov 2013 13:32:02 -  1.15
> +++ games/morse/morse.c   9 Jun 2015 17:28:09 -
> @@ -120,7 +120,7 @@ main(int argc, char *argv[])
>   case 's':
>   sflag = 1;
>   break;
> - case '?': case 'h':
> + case 'h':
>   default:
>   fprintf(stderr, "usage: morse [-d | -s] [string 
> ...]\n");
>   exit(1);
> Index: games/number/number.c
> ===
> RCS file: /cvs/src/games/number/number.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 number.c
> --- games/number/number.c 27 Oct 2009 23:59:26 -  1.14
> +++ games/number/number.c 9 Jun 2015 17:28:09 -
> @@ -85

Re: [patch] libc/locale/setlocale.c: remove unused arg

2015-06-09 Thread Stefan Sperling
On Tue, Jun 09, 2015 at 07:47:21PM +0200, Sébastien Marie wrote:
> Hi,
> 
> I start reading libc/locale code in order to understanding it.
> 
> Here a patch to remove an unused argument "isspecial" from static
> function "load_locale_sub".
> 
> The function is called once, with isspecial=0, and the argument isn't
> used in function's body.
> 
> Any comments ?

Committed, thanks.



[patch] libc/locale/setlocale.c: remove unused arg

2015-06-09 Thread Sébastien Marie
Hi,

I start reading libc/locale code in order to understanding it.

Here a patch to remove an unused argument "isspecial" from static
function "load_locale_sub".

The function is called once, with isspecial=0, and the argument isn't
used in function's body.

Any comments ?
-- 
Sébastien Marie


Index: locale/setlocale.c
===
RCS file: /cvs/src/lib/libc/locale/setlocale.c,v
retrieving revision 1.20
diff -u -p -r1.20 setlocale.c
--- locale/setlocale.c  28 Aug 2013 16:53:34 -  1.20
+++ locale/setlocale.c  9 Jun 2015 17:40:42 -
@@ -77,7 +77,7 @@ static char current_locale_string[_LC_LA
 
 static char*currentlocale(void);
 static void revert_to_default(int);
-static int load_locale_sub(int, const char *, int);
+static int load_locale_sub(int, const char *);
 static char*loadlocale(int);
 static const char *__get_locale_env(int);
 
@@ -238,7 +238,7 @@ set_lc_messages_locale(const char *locna
 }
 
 static int
-load_locale_sub(int category, const char *locname, int isspecial)
+load_locale_sub(int category, const char *locname)
 {
/* check for the default locales */
if (!strcmp(new_categories[category], "C") ||
@@ -278,7 +278,7 @@ loadlocale(int category)
current_categories[category]) == 0)
return (current_categories[category]);
 
-   if (!load_locale_sub(category, new_categories[category], 0)) {
+   if (!load_locale_sub(category, new_categories[category])) {
(void)strlcpy(current_categories[category],
new_categories[category], 
sizeof(current_categories[category]));
return current_categories[category];



clean the tree from superfluous "case '?': /* FALLTHROUGH */"

2015-06-09 Thread Benjamin Baier
Hello tech@

delete "case '?': /* FALLTHROUGH */" where it is already handled 
by "default: usage();".

Rarely also adjust getopt(3)'s optstring.



Index: bin/pax/options.c
===
RCS file: /cvs/src/bin/pax/options.c,v
retrieving revision 1.91
diff -u -p -r1.91 options.c
--- bin/pax/options.c   18 May 2015 20:26:16 -  1.91
+++ bin/pax/options.c   9 Jun 2015 17:27:56 -
@@ -1307,7 +1307,6 @@ cpio_options(int argc, char **argv)
 */
frmt = &(fsub[F_OCPIO]);
break;
-   case '?':
default:
cpio_usage();
break;
Index: games/arithmetic/arithmetic.c
===
RCS file: /cvs/src/games/arithmetic/arithmetic.c,v
retrieving revision 1.18
diff -u -p -r1.18 arithmetic.c
--- games/arithmetic/arithmetic.c   29 Aug 2013 20:22:09 -  1.18
+++ games/arithmetic/arithmetic.c   9 Jun 2015 17:28:09 -
@@ -116,7 +116,6 @@ main(int argc, char *argv[])
if ((rangemax = atoi(optarg)) <= 0)
errx(1, "invalid range.");
break;
-   case '?':
case 'h':
default:
usage();
Index: games/banner/banner.c
===
RCS file: /cvs/src/games/banner/banner.c,v
retrieving revision 1.17
diff -u -p -r1.17 banner.c
--- games/banner/banner.c   10 Feb 2015 13:50:58 -  1.17
+++ games/banner/banner.c   9 Jun 2015 17:28:09 -
@@ -1030,7 +1030,7 @@ main(int argc, char *argv[])
if (width <= 0 || width > DWIDTH)
errx(1, "illegal argument for -w option");
break;
-   case '?': case 'h':
+   case 'h':
default:
(void)fprintf(stderr,
"usage: banner [-w width] message ...\n");
Index: games/boggle/boggle/bog.c
===
RCS file: /cvs/src/games/boggle/boggle/bog.c,v
retrieving revision 1.24
diff -u -p -r1.24 bog.c
--- games/boggle/boggle/bog.c   4 Dec 2014 06:12:33 -   1.24
+++ games/boggle/boggle/bog.c   9 Jun 2015 17:28:09 -
@@ -117,7 +117,6 @@ main(int argc, char *argv[])
if ((minlength = atoi(optarg)) < 3)
errx(1, "min word length must be > 2");
break;
-   case '?':
default:
usage();
}
Index: games/cribbage/crib.c
===
RCS file: /cvs/src/games/cribbage/crib.c,v
retrieving revision 1.18
diff -u -p -r1.18 crib.c
--- games/cribbage/crib.c   12 Mar 2015 02:19:10 -  1.18
+++ games/cribbage/crib.c   9 Jun 2015 17:28:09 -
@@ -63,7 +63,6 @@ main(int argc, char *argv[])
case 'r':
rflag = TRUE;
break;
-   case '?':
default:
(void) fprintf(stderr, "usage: cribbage [-emqr]\n");
exit(1);
Index: games/factor/factor.c
===
RCS file: /cvs/src/games/factor/factor.c,v
retrieving revision 1.19
diff -u -p -r1.19 factor.c
--- games/factor/factor.c   27 Oct 2009 23:59:24 -  1.19
+++ games/factor/factor.c   9 Jun 2015 17:28:09 -
@@ -89,7 +89,6 @@ main(int argc, char *argv[])
 
while ((ch = getopt(argc, argv, "")) != -1)
switch (ch) {
-   case '?':
default:
usage();
}
Index: games/fish/fish.c
===
RCS file: /cvs/src/games/fish/fish.c,v
retrieving revision 1.17
diff -u -p -r1.17 fish.c
--- games/fish/fish.c   18 Feb 2015 23:20:45 -  1.17
+++ games/fish/fish.c   9 Jun 2015 17:28:09 -
@@ -92,7 +92,6 @@ main(int argc, char *argv[])
case 'p':
promode = 1;
break;
-   case '?':
case 'h':
default:
usage();
Index: games/fortune/fortune/fortune.c
===
RCS file: /cvs/src/games/fortune/fortune/fortune.c,v
retrieving revision 1.42
diff -u -p -r1.42 fortune.c
--- games/fortune/fortune/fortune.c 6 Feb 2015 10:50:48 -   1.42
+++ games/fortune/fortune/fortune.c 9 Jun 2015 17:28:09 -
@@ -303,7 +303,6 @@ getargs(int argc, char *argv[])
case 'i':   

Re: SNMP: support new no-route pf counter

2015-06-09 Thread Mike Belopuhov
On Tue, Jun 09, 2015 at 17:52 +0200, Mike Belopuhov wrote:
> Hi,
> 
> Please review SNMP bits for the new 'no-route' pf state insertion
> failure counter.  Any improvements to the MIB description?  Here's
> what I mean by "no target addresses were available": for instance,
> with such ruleset:
> 
>   table  persist
>   pass in on vmx0 inet proto tcp to port 80 route-to 
> 
> there's no target that pf can select for route-to since ''
> doesn't contain any entries and therefore can't complete state
> creation/insertion.
> 
> OKs?
>

sthen@ has pointed out that I've forgotten to change the timestamp
and add a description of changes.  Now with those bits in place.
He has also requested to run smilint and I'm happy to report that
it passes successfully.

diff --git share/snmp/OPENBSD-PF-MIB.txt share/snmp/OPENBSD-PF-MIB.txt
index ae96829..04b56c9 100644
--- share/snmp/OPENBSD-PF-MIB.txt
+++ share/snmp/OPENBSD-PF-MIB.txt
@@ -31,20 +31,22 @@ IMPORTS

MODULE-COMPLIANCE, OBJECT-GROUP
FROM SNMPv2-CONF;
 
 pfMIBObjects MODULE-IDENTITY
-LAST-UPDATED "201308310446Z"
+LAST-UPDATED "201506091728Z"
 ORGANIZATION "OpenBSD"
 CONTACT-INFO "
   Author: Joel Knight
   email:  knight.j...@gmail.com
   www:http://www.packetmischief.ca/openbsd-snmp-mibs/
  "
 DESCRIPTION "The MIB module for gathering information from
OpenBSD's packet filter.
 "
+REVISION "201506091728Z"
+DESCRIPTION "Add separate counter for failed 'route-to' applications"
 REVISION "201308310446Z"
 DESCRIPTION "Add pf(4) table byte/packet counters for 'match' rules"
 REVISION "201302242033Z"
 DESCRIPTION "Add separate counter for failed translations"
 REVISION "20120126Z"
@@ -249,10 +251,18 @@ pfCntTranslate OBJECT-TYPE
 DESCRIPTION
"The number of packets that were dropped because network address
 translation was requested and no unused port was available."
 ::= { pfCounters 16 }
 
+pfCntNoRoute OBJECT-TYPE
+SYNTAX  Counter64
+MAX-ACCESS  read-only
+STATUS  current
+DESCRIPTION
+   "The number of packets that were dropped because policy based routing
+was requested but no target addresses were available."
+::= { pfCounters 17 }
 
 -- pfStateTable
 
 pfStateCount OBJECT-TYPE
 SYNTAX  Unsigned32
> diff --git usr.sbin/snmpd/mib.c usr.sbin/snmpd/mib.c
> index 8e4d98f..c8a8fa0 100644
> --- usr.sbin/snmpd/mib.c
> +++ usr.sbin/snmpd/mib.c
> @@ -1449,10 +1449,11 @@ static struct oid openbsd_mib[] = {
>   { MIB(pfCntStateInsert),OID_RD, mib_pfcounters },
>   { MIB(pfCntStateLimit), OID_RD, mib_pfcounters },
>   { MIB(pfCntSrcLimit),   OID_RD, mib_pfcounters },
>   { MIB(pfCntSynproxy),   OID_RD, mib_pfcounters },
>   { MIB(pfCntTranslate),  OID_RD, mib_pfcounters },
> + { MIB(pfCntNoRoute),OID_RD, mib_pfcounters },
>   { MIB(pfStateCount),OID_RD, mib_pfscounters },
>   { MIB(pfStateSearches), OID_RD, mib_pfscounters },
>   { MIB(pfStateInserts),  OID_RD, mib_pfscounters },
>   { MIB(pfStateRemovals), OID_RD, mib_pfscounters },
>   { MIB(pfLogIfName), OID_RD, mib_pflogif },
> @@ -1705,11 +1706,12 @@ mib_pfcounters(struct oid *oid, struct ber_oid *o, 
> struct ber_element **elm)
>   { 11, &s.counters[PFRES_BADSTATE] },
>   { 12, &s.counters[PFRES_STATEINS] },
>   { 13, &s.counters[PFRES_MAXSTATES] },
>   { 14, &s.counters[PFRES_SRCLIMIT] },
>   { 15, &s.counters[PFRES_SYNPROXY] },
> - { 16, &s.counters[PFRES_TRANSLATE] }
> + { 16, &s.counters[PFRES_TRANSLATE] },
> + { 17, &s.counters[PFRES_NOROUTE] }
>   };
>  
>   if (pf_get_stats(&s))
>   return (-1);
>  
> diff --git usr.sbin/snmpd/mib.h usr.sbin/snmpd/mib.h
> index 4fff5ec..5e87e4d 100644
> --- usr.sbin/snmpd/mib.h
> +++ usr.sbin/snmpd/mib.h
> @@ -488,10 +488,11 @@
>  #define MIB_pfCntStateInsert MIB_pfCounters, 12
>  #define MIB_pfCntStateLimit  MIB_pfCounters, 13
>  #define MIB_pfCntSrcLimitMIB_pfCounters, 14
>  #define MIB_pfCntSynproxyMIB_pfCounters, 15
>  #define MIB_pfCntTranslate   MIB_pfCounters, 16
> +#define MIB_pfCntNoRoute MIB_pfCounters, 17
>  #define MIB_pfStateTable MIB_pfMIBObjects, 3
>  #define MIB_pfStateCount MIB_pfStateTable, 1
>  #define MIB_pfStateSearches  MIB_pfStateTable, 2
>  #define MIB_pfStateInserts   MIB_pfStateTable, 3
>  #define MIB_pfStateRemovals  MIB_pfStateTable, 4
> @@ -1055,10 +1056,11 @@
>   { MIBDECL(pfCntStateInsert) },  \
>   { MIBDECL(pfCntStateLimit) },   \
>   { MIBDECL(pfCntSrcLimit) }, 

don't error for duplicate identical type definitions

2015-06-09 Thread Miod Vallat
Our current base compiler will abort with an error if it finds a typedef
being redefined, even if the new definition is no different from the one
it knows about.

Allowing this kind of type redefinition is a new C11 feature, which some
third-party software has already started using. Moreover, recent gcc
compilers will indeed accept this, even in non-c11 mode, with an
optional warning.

Consider the following testcase:
$ cat type.c
typedef unsigned char uint8_t;
typedef unsigned char char_t;

/* the following should not cause errors */
typedef unsigned char uint8_t;
typedef char_t uint8_t;

/* the following should cause errors */
typedef unsigned int uint8_t;
$

With the current in-tree compiler, it will produce these errors:

type.c:5: error: redefinition of typedef 'uint8_t'
type.c:1: error: previous declaration of 'uint8_t' was here
type.c:6: error: redefinition of typedef 'uint8_t'
type.c:5: error: previous declaration of 'uint8_t' was here
type.c:9: error: conflicting types for 'uint8_t'
type.c:6: error: previous declaration of 'uint8_t' was here

with the diff I am proposing below, it will only produce these errors:

type.c:9: error: conflicting types for 'uint8_t'
type.c:6: error: previous declaration of 'uint8_t' was here

and adding -pedantic will produce these warnings:

type.c:5: warning: redefinition of typedef 'uint8_t'
type.c:6: warning: redefinition of typedef 'uint8_t'

Any objections?

Index: c-decl.c
===
RCS file: /OpenBSD/src/gnu/gcc/gcc/c-decl.c,v
retrieving revision 1.2
diff -u -p -r1.2 c-decl.c
--- c-decl.c29 Apr 2010 18:37:37 -  1.2
+++ c-decl.c2 May 2015 14:35:28 -
@@ -1285,9 +1285,17 @@ diagnose_mismatched_decls (tree newdecl,
   if (DECL_IN_SYSTEM_HEADER (newdecl) || DECL_IN_SYSTEM_HEADER (olddecl))
return true;  /* Allow OLDDECL to continue in use.  */
 
-  error ("redefinition of typedef %q+D", newdecl);
-  locate_old_decl (olddecl, error);
-  return false;
+  if (pedantic)
+   {
+ pedwarn ("redefinition of typedef %q+D", newdecl);
+ if (flag_pedantic_errors)
+   {
+ locate_old_decl (olddecl, error);
+ return false;
+   }
+   }
+
+  return true;
 }
 
   /* Function declarations can either be 'static' or 'extern' (no



Re: pf: increment rule counters after successful state insertion

2015-06-09 Thread Mike Belopuhov
On Tue, Jun 09, 2015 at 18:11 +0200, Mike Belopuhov wrote:
> Hi,
> 
> I was surprised to see 'State Creations' rule counter go up when

I've just realised that I might have been a bit too vague in my
description.

> no real state creation happens.

Should read: when state creation/insertion fails.

> This is because we increment all
> counters too early, but then don't decrement 'states_tot'

We don't decrement it in case of a failure.

> which
> is a total number of states created by the rule.

> Not entirely
> sure why was it done but I see no reason for it to stay this way.
> 
> IMO total has to increase only when we succeed inserting the state
> into the table.
> 
> This has an additional benefit of making error handling simpler:
> we don't need to unroll our counter increments if we do them right
> after pf_state_insert.
> 
> OK?
> 
> diff --git sys/net/pf.c sys/net/pf.c
> index 4192f14..efa1388 100644
> --- sys/net/pf.c
> +++ sys/net/pf.c
> @@ -273,20 +273,10 @@ struct pf_pool_limit pf_pool_limits[PF_LIMIT_MAX] = {
>   }   \
>   SLIST_FOREACH(mrm, &s->match_rules, entry)  \
>   mrm->r->states_cur++;   \
>   } while (0)
>  
> -#define STATE_DEC_COUNTERS(s)\
> - do {\
> - struct pf_rule_item *mrm;   \
> - if (s->anchor.ptr != NULL)  \
> - s->anchor.ptr->states_cur--;\
> - s->rule.ptr->states_cur--;  \
> - SLIST_FOREACH(mrm, &s->match_rules, entry)  \
> - mrm->r->states_cur--;   \
> - } while (0)
> -
>  static __inline int pf_src_compare(struct pf_src_node *, struct pf_src_node 
> *);
>  static __inline int pf_state_compare_key(struct pf_state_key *,
>   struct pf_state_key *);
>  static __inline int pf_state_compare_id(struct pf_state *,
>   struct pf_state *);
> @@ -3464,11 +3454,10 @@ pf_create_state(struct pf_pdesc *pd, struct pf_rule 
> *r, struct pf_rule *a,
>   }
>   s->rule.ptr = r;
>   s->anchor.ptr = a;
>   s->natrule.ptr = nr;
>   memcpy(&s->match_rules, rules, sizeof(s->match_rules));
> - STATE_INC_COUNTERS(s);
>   if (r->allow_opts)
>   s->state_flags |= PFSTATE_ALLOWOPTS;
>   if (r->rule_flag & PFRULE_STATESLOPPY)
>   s->state_flags |= PFSTATE_SLOPPY;
>   if (r->rule_flag & PFRULE_PFLOW)
> @@ -3591,10 +3580,12 @@ pf_create_state(struct pf_pdesc *pd, struct pf_rule 
> *r, struct pf_rule *a,
>   REASON_SET(&reason, PFRES_STATEINS);
>   goto csfailed;
>   } else
>   *sm = s;
>  
> + STATE_INC_COUNTERS(s);
> +
>   if (tag > 0) {
>   pf_tag_ref(tag);
>   s->tag = tag;
>   }
>   if (pd->proto == IPPROTO_TCP && (th->th_flags & (TH_SYN|TH_ACK)) ==
> @@ -3621,21 +3612,17 @@ pf_create_state(struct pf_pdesc *pd, struct pf_rule 
> *r, struct pf_rule *a,
>  
>  csfailed:
>   if (s) {
>   pf_normalize_tcp_cleanup(s);/* safe even w/o init */
>   pf_src_tree_remove_state(s);
> + pool_put(&pf_state_pl, s);
>   }
>  
>   for (i = 0; i < PF_SN_MAX; i++)
>   if (sns[i] != NULL)
>   pf_remove_src_node(sns[i]);
>  
> - if (s) {
> - STATE_DEC_COUNTERS(s);
> - pool_put(&pf_state_pl, s);
> - }
> -
>   return (PF_DROP);
>  }
>  
>  int
>  pf_translate(struct pf_pdesc *pd, struct pf_addr *saddr, u_int16_t sport,



pf: increment rule counters after successful state insertion

2015-06-09 Thread Mike Belopuhov
Hi,

I was surprised to see 'State Creations' rule counter go up when
no real state creation happens.  This is because we increment all
counters too early, but then don't decrement 'states_tot' which
is a total number of states created by the rule.  Not entirely
sure why was it done but I see no reason for it to stay this way.

IMO total has to increase only when we succeed inserting the state
into the table.

This has an additional benefit of making error handling simpler:
we don't need to unroll our counter increments if we do them right
after pf_state_insert.

OK?

diff --git sys/net/pf.c sys/net/pf.c
index 4192f14..efa1388 100644
--- sys/net/pf.c
+++ sys/net/pf.c
@@ -273,20 +273,10 @@ struct pf_pool_limit pf_pool_limits[PF_LIMIT_MAX] = {
}   \
SLIST_FOREACH(mrm, &s->match_rules, entry)  \
mrm->r->states_cur++;   \
} while (0)
 
-#define STATE_DEC_COUNTERS(s)  \
-   do {\
-   struct pf_rule_item *mrm;   \
-   if (s->anchor.ptr != NULL)  \
-   s->anchor.ptr->states_cur--;\
-   s->rule.ptr->states_cur--;  \
-   SLIST_FOREACH(mrm, &s->match_rules, entry)  \
-   mrm->r->states_cur--;   \
-   } while (0)
-
 static __inline int pf_src_compare(struct pf_src_node *, struct pf_src_node *);
 static __inline int pf_state_compare_key(struct pf_state_key *,
struct pf_state_key *);
 static __inline int pf_state_compare_id(struct pf_state *,
struct pf_state *);
@@ -3464,11 +3454,10 @@ pf_create_state(struct pf_pdesc *pd, struct pf_rule *r, 
struct pf_rule *a,
}
s->rule.ptr = r;
s->anchor.ptr = a;
s->natrule.ptr = nr;
memcpy(&s->match_rules, rules, sizeof(s->match_rules));
-   STATE_INC_COUNTERS(s);
if (r->allow_opts)
s->state_flags |= PFSTATE_ALLOWOPTS;
if (r->rule_flag & PFRULE_STATESLOPPY)
s->state_flags |= PFSTATE_SLOPPY;
if (r->rule_flag & PFRULE_PFLOW)
@@ -3591,10 +3580,12 @@ pf_create_state(struct pf_pdesc *pd, struct pf_rule *r, 
struct pf_rule *a,
REASON_SET(&reason, PFRES_STATEINS);
goto csfailed;
} else
*sm = s;
 
+   STATE_INC_COUNTERS(s);
+
if (tag > 0) {
pf_tag_ref(tag);
s->tag = tag;
}
if (pd->proto == IPPROTO_TCP && (th->th_flags & (TH_SYN|TH_ACK)) ==
@@ -3621,21 +3612,17 @@ pf_create_state(struct pf_pdesc *pd, struct pf_rule *r, 
struct pf_rule *a,
 
 csfailed:
if (s) {
pf_normalize_tcp_cleanup(s);/* safe even w/o init */
pf_src_tree_remove_state(s);
+   pool_put(&pf_state_pl, s);
}
 
for (i = 0; i < PF_SN_MAX; i++)
if (sns[i] != NULL)
pf_remove_src_node(sns[i]);
 
-   if (s) {
-   STATE_DEC_COUNTERS(s);
-   pool_put(&pf_state_pl, s);
-   }
-
return (PF_DROP);
 }
 
 int
 pf_translate(struct pf_pdesc *pd, struct pf_addr *saddr, u_int16_t sport,



SNMP: support new no-route pf counter

2015-06-09 Thread Mike Belopuhov
Hi,

Please review SNMP bits for the new 'no-route' pf state insertion
failure counter.  Any improvements to the MIB description?  Here's
what I mean by "no target addresses were available": for instance,
with such ruleset:

  table  persist
  pass in on vmx0 inet proto tcp to port 80 route-to 

there's no target that pf can select for route-to since ''
doesn't contain any entries and therefore can't complete state
creation/insertion.

OKs?

diff --git share/snmp/OPENBSD-PF-MIB.txt share/snmp/OPENBSD-PF-MIB.txt
index ae96829..3bc2eb9 100644
--- share/snmp/OPENBSD-PF-MIB.txt
+++ share/snmp/OPENBSD-PF-MIB.txt
@@ -249,10 +249,18 @@ pfCntTranslate OBJECT-TYPE
 DESCRIPTION
"The number of packets that were dropped because network address
 translation was requested and no unused port was available."
 ::= { pfCounters 16 }
 
+pfCntNoRoute OBJECT-TYPE
+SYNTAX  Counter64
+MAX-ACCESS  read-only
+STATUS  current
+DESCRIPTION
+   "The number of packets that were dropped because policy based routing
+was requested but no target addresses were available."
+::= { pfCounters 17 }
 
 -- pfStateTable
 
 pfStateCount OBJECT-TYPE
 SYNTAX  Unsigned32
diff --git usr.sbin/snmpd/mib.c usr.sbin/snmpd/mib.c
index 8e4d98f..c8a8fa0 100644
--- usr.sbin/snmpd/mib.c
+++ usr.sbin/snmpd/mib.c
@@ -1449,10 +1449,11 @@ static struct oid openbsd_mib[] = {
{ MIB(pfCntStateInsert),OID_RD, mib_pfcounters },
{ MIB(pfCntStateLimit), OID_RD, mib_pfcounters },
{ MIB(pfCntSrcLimit),   OID_RD, mib_pfcounters },
{ MIB(pfCntSynproxy),   OID_RD, mib_pfcounters },
{ MIB(pfCntTranslate),  OID_RD, mib_pfcounters },
+   { MIB(pfCntNoRoute),OID_RD, mib_pfcounters },
{ MIB(pfStateCount),OID_RD, mib_pfscounters },
{ MIB(pfStateSearches), OID_RD, mib_pfscounters },
{ MIB(pfStateInserts),  OID_RD, mib_pfscounters },
{ MIB(pfStateRemovals), OID_RD, mib_pfscounters },
{ MIB(pfLogIfName), OID_RD, mib_pflogif },
@@ -1705,11 +1706,12 @@ mib_pfcounters(struct oid *oid, struct ber_oid *o, 
struct ber_element **elm)
{ 11, &s.counters[PFRES_BADSTATE] },
{ 12, &s.counters[PFRES_STATEINS] },
{ 13, &s.counters[PFRES_MAXSTATES] },
{ 14, &s.counters[PFRES_SRCLIMIT] },
{ 15, &s.counters[PFRES_SYNPROXY] },
-   { 16, &s.counters[PFRES_TRANSLATE] }
+   { 16, &s.counters[PFRES_TRANSLATE] },
+   { 17, &s.counters[PFRES_NOROUTE] }
};
 
if (pf_get_stats(&s))
return (-1);
 
diff --git usr.sbin/snmpd/mib.h usr.sbin/snmpd/mib.h
index 4fff5ec..5e87e4d 100644
--- usr.sbin/snmpd/mib.h
+++ usr.sbin/snmpd/mib.h
@@ -488,10 +488,11 @@
 #define MIB_pfCntStateInsert   MIB_pfCounters, 12
 #define MIB_pfCntStateLimitMIB_pfCounters, 13
 #define MIB_pfCntSrcLimit  MIB_pfCounters, 14
 #define MIB_pfCntSynproxy  MIB_pfCounters, 15
 #define MIB_pfCntTranslate MIB_pfCounters, 16
+#define MIB_pfCntNoRoute   MIB_pfCounters, 17
 #define MIB_pfStateTable   MIB_pfMIBObjects, 3
 #define MIB_pfStateCount   MIB_pfStateTable, 1
 #define MIB_pfStateSearchesMIB_pfStateTable, 2
 #define MIB_pfStateInserts MIB_pfStateTable, 3
 #define MIB_pfStateRemovalsMIB_pfStateTable, 4
@@ -1055,10 +1056,11 @@
{ MIBDECL(pfCntStateInsert) },  \
{ MIBDECL(pfCntStateLimit) },   \
{ MIBDECL(pfCntSrcLimit) }, \
{ MIBDECL(pfCntSynproxy) }, \
{ MIBDECL(pfCntTranslate) },\
+   { MIBDECL(pfCntNoRoute) },  \
{ MIBDECL(pfStateTable) },  \
{ MIBDECL(pfStateCount) },  \
{ MIBDECL(pfStateSearches) },   \
{ MIBDECL(pfStateInserts) },\
{ MIBDECL(pfStateRemovals) },   \



Re: pfctl -ss -R

2015-06-09 Thread Sebastian Benoit
Mike Belopuhov(m...@belopuhov.com) on 2015.06.09 16:23:04 +0200:
> Hi,
> 
> Any idea why don't we support filtering the show states output
> by the associated rule number?

indeed, why not?
 
> Diff below works fine here, OK?

ok!
 
> Index: pfctl.c
> ===
> RCS file: /home/cvs/src/sbin/pfctl/pfctl.c,v
> retrieving revision 1.329
> diff -u -p -r1.329 pfctl.c
> --- pfctl.c   16 Jan 2015 06:40:00 -  1.329
> +++ pfctl.c   9 Jun 2015 14:14:49 -
> @@ -84,7 +84,7 @@ void pfctl_print_rule_counters(struct p
>  int   pfctl_show_rules(int, char *, int, enum pfctl_show, char *, int, int,
>   long);
>  int   pfctl_show_src_nodes(int, int);
> -int   pfctl_show_states(int, const char *, int);
> +int   pfctl_show_states(int, const char *, int, long);
>  int   pfctl_show_status(int, int);
>  int   pfctl_show_timeouts(int, int);
>  int   pfctl_show_limits(int, int);
> @@ -945,7 +945,7 @@ done:
>  }
>  
>  int
> -pfctl_show_states(int dev, const char *iface, int opts)
> +pfctl_show_states(int dev, const char *iface, int opts, long shownr)
>  {
>   struct pfioc_states ps;
>   struct pfsync_state *p;
> @@ -985,7 +985,8 @@ pfctl_show_states(int dev, const char *i
>   pfctl_print_title("STATES:");
>   dotitle = 0;
>   }
> - print_state(p, opts);
> + if (shownr < 0 || ntohl(p->rule) == shownr)
> + print_state(p, opts);
>   }
>  done:
>   free(inbuf);
> @@ -2309,7 +2310,7 @@ main(int argc, char *argv[])
>   opts & PF_OPT_VERBOSE2);
>   break;
>   case 's':
> - pfctl_show_states(dev, ifaceopt, opts);
> + pfctl_show_states(dev, ifaceopt, opts, shownr);
>   break;
>   case 'S':
>   pfctl_show_src_nodes(dev, opts);
> @@ -2329,7 +2330,7 @@ main(int argc, char *argv[])
>  
>   pfctl_show_rules(dev, path, opts, 0, anchorname,
>   0, 0, -1);
> - pfctl_show_states(dev, ifaceopt, opts);
> + pfctl_show_states(dev, ifaceopt, opts, -1);
>   pfctl_show_src_nodes(dev, opts);
>   pfctl_show_status(dev, opts);
>   pfctl_show_rules(dev, path, opts, 1, anchorname,
> 

-- 



pfctl -ss -R

2015-06-09 Thread Mike Belopuhov
Hi,

Any idea why don't we support filtering the show states output
by the associated rule number?

Diff below works fine here, OK?

Index: pfctl.c
===
RCS file: /home/cvs/src/sbin/pfctl/pfctl.c,v
retrieving revision 1.329
diff -u -p -r1.329 pfctl.c
--- pfctl.c 16 Jan 2015 06:40:00 -  1.329
+++ pfctl.c 9 Jun 2015 14:14:49 -
@@ -84,7 +84,7 @@ void   pfctl_print_rule_counters(struct p
 int pfctl_show_rules(int, char *, int, enum pfctl_show, char *, int, int,
long);
 int pfctl_show_src_nodes(int, int);
-int pfctl_show_states(int, const char *, int);
+int pfctl_show_states(int, const char *, int, long);
 int pfctl_show_status(int, int);
 int pfctl_show_timeouts(int, int);
 int pfctl_show_limits(int, int);
@@ -945,7 +945,7 @@ done:
 }
 
 int
-pfctl_show_states(int dev, const char *iface, int opts)
+pfctl_show_states(int dev, const char *iface, int opts, long shownr)
 {
struct pfioc_states ps;
struct pfsync_state *p;
@@ -985,7 +985,8 @@ pfctl_show_states(int dev, const char *i
pfctl_print_title("STATES:");
dotitle = 0;
}
-   print_state(p, opts);
+   if (shownr < 0 || ntohl(p->rule) == shownr)
+   print_state(p, opts);
}
 done:
free(inbuf);
@@ -2309,7 +2310,7 @@ main(int argc, char *argv[])
opts & PF_OPT_VERBOSE2);
break;
case 's':
-   pfctl_show_states(dev, ifaceopt, opts);
+   pfctl_show_states(dev, ifaceopt, opts, shownr);
break;
case 'S':
pfctl_show_src_nodes(dev, opts);
@@ -2329,7 +2330,7 @@ main(int argc, char *argv[])
 
pfctl_show_rules(dev, path, opts, 0, anchorname,
0, 0, -1);
-   pfctl_show_states(dev, ifaceopt, opts);
+   pfctl_show_states(dev, ifaceopt, opts, -1);
pfctl_show_src_nodes(dev, opts);
pfctl_show_status(dev, opts);
pfctl_show_rules(dev, path, opts, 1, anchorname,



Re: [Patch] httpd - don't leak fcgi file descriptors

2015-06-09 Thread Joerg Jung
On Tue, Jun 09, 2015 at 07:46:15AM +, Florian Obser wrote:
> On Mon, Jun 08, 2015 at 09:17:41PM +0200, Claudio Jeker wrote:
> > On Mon, Jun 08, 2015 at 09:12:32PM +0200, Joerg Jung wrote:
> > > On Tue, Jun 02, 2015 at 05:47:47PM +0200, Claudio Jeker wrote:
> > > > On Tue, Jun 02, 2015 at 01:50:35PM +0200, Joerg Jung wrote:
> > > > >
> > > > > > Am 01.06.2015 um 01:25 schrieb Todd Mortimer :
> > > > > >
> > > > > > I agree that my patch is more of a workaround, and it would be
> > > > > > better to track down how it is that the client is being passed to
> > > > > > server_fcgi with an open socket. I was going this way when I started
> > > > > > looking at the source, but then I saw that clt->clt_srvevb and
> > > > > > clt->clt_srvbev get the same treatment (free if not null, then
> > > > > > reassign) at the same spot in server_fcgi(), and I figured if it
> > > > > > was good enough for clt_srvevb and clt_srvbev, why not for clt_fd?
> > > > >
> > > > > Yes, you are right. I think your proposed diff is correct.
> > > > > I would like to commit it, if anyone is willing to give OK.
> > > >
> > > > This feels to me more like a workaround. Since what happens is that a
> > > > connection is either reused without cleanup or we call the connection
> > > > establishment multiple time for the same client.
> > > > relayd had a similar issue that I fixed lately. One of the issues is 
> > > > that
> > > > event callbacks can be called when you don't really expect them to be
> > > > called.
> > > 
> > > Yes, workaround was my first impression as well.  But after staring at
> > > the code for a while, I think  fixing it in the "right way" seems not
> > > trivial and involves several changes.
> > > 
> > > So, since the diff below is simple and goes along with the style of the
> > > existing code, AND fixes an actual leak, I would suggest to commit it
> > > for now, until someone comes up with something better?
> > 
> > Sure. Fine with me. Wondering if the -1 check is needed. IIRC close(-1);
> > is save. Anyway you want to add a space after the if.
> >  
> 
> OK for me as well. Please go ahead. I was hoping to look into it and
> find the "correct" fix but I didn't get around to it. So this
> workaround is better than nothing I guess.

Committed, thanks.

Regards,
Joerg



Re: jail_bin_add: script to add binary and libs to chroot

2015-06-09 Thread Marc Espie
On Mon, Jun 08, 2015 at 03:37:27PM +0200, Landry Breuil wrote:
> On Mon, Jun 08, 2015 at 02:59:28PM +0200, Marc Espie wrote:
> > On Mon, Jun 08, 2015 at 01:46:17AM -0400, dan mclaughlin wrote:
> > > i figure this should be useful to some.
> > > any nits welcome.
> > 
> > Unfortunately, this will become increasingly useless in
> > gtk-land.
> > 
> > Compare ldd firefox vs a ktrace of the running binary... :(
> 
> Totally pointless example - firefox is a very specific case, in that
> case you want ldd /usr/local/lib/firefox-*/libxul.so.*  i dont know
> of many gtk applications doing this (bundling everything in a dlopen'ed
> library with tons of linking hacks to improve startup times...)

I took firefox because it is the most blatant example, but Antoine told
me about it in general before I took a look at firefox.



Re: [Patch] httpd - don't leak fcgi file descriptors

2015-06-09 Thread Florian Obser
On Mon, Jun 08, 2015 at 09:17:41PM +0200, Claudio Jeker wrote:
> On Mon, Jun 08, 2015 at 09:12:32PM +0200, Joerg Jung wrote:
> > On Tue, Jun 02, 2015 at 05:47:47PM +0200, Claudio Jeker wrote:
> > > On Tue, Jun 02, 2015 at 01:50:35PM +0200, Joerg Jung wrote:
> > > >
> > > > > Am 01.06.2015 um 01:25 schrieb Todd Mortimer :
> > > > >
> > > > > I agree that my patch is more of a workaround, and it would be
> > > > > better to track down how it is that the client is being passed to
> > > > > server_fcgi with an open socket. I was going this way when I started
> > > > > looking at the source, but then I saw that clt->clt_srvevb and
> > > > > clt->clt_srvbev get the same treatment (free if not null, then
> > > > > reassign) at the same spot in server_fcgi(), and I figured if it
> > > > > was good enough for clt_srvevb and clt_srvbev, why not for clt_fd?
> > > >
> > > > Yes, you are right. I think your proposed diff is correct.
> > > > I would like to commit it, if anyone is willing to give OK.
> > >
> > > This feels to me more like a workaround. Since what happens is that a
> > > connection is either reused without cleanup or we call the connection
> > > establishment multiple time for the same client.
> > > relayd had a similar issue that I fixed lately. One of the issues is that
> > > event callbacks can be called when you don't really expect them to be
> > > called.
> > 
> > Yes, workaround was my first impression as well.  But after staring at
> > the code for a while, I think  fixing it in the "right way" seems not
> > trivial and involves several changes.
> > 
> > So, since the diff below is simple and goes along with the style of the
> > existing code, AND fixes an actual leak, I would suggest to commit it
> > for now, until someone comes up with something better?
> 
> Sure. Fine with me. Wondering if the -1 check is needed. IIRC close(-1);
> is save. Anyway you want to add a space after the if.
>  

OK for me as well. Please go ahead. I was hoping to look into it and
find the "correct" fix but I didn't get around to it. So this
workaround is better than nothing I guess.

-- 
I'm not entirely sure you are real.