Re: readdir man page
On Thu, Feb 02, 2012 at 09:50:45PM -0700, Philip Guenther wrote: The current man page doesn't claim that errno is always set but one might reasonably assume that it is: certainly, it seems a horribly easy way of introducing bugs (at least for idiots such as myself). [...] [Christiano] Although I like your intention, I think the problem is in the later part: The readdir() and readdir_r() functions may also fail and set errno for any of the errors specified for the routine getdirentries(2). Maybe this could be clarified ? Lets see what others think. I'll propose a third option: I think the directory(3) manpage would be clearer if the DESCRIPTION section was unwoven to have separate DESCRIPTION and RETURN VALUES section. Explicit wording that errno is not set when end of directory is reached and that programs that want to tell apart end-of-directory and errors must set errno to zero should be added. I think this would make a lot of sense. I also think readdir() should set errno if it detects an invalid seekdir(). EINVAL seems correct. This seems like a good idea overall, but I have one minor worry. Does any other Unix do this? If not, it may introduce another subtle portability bug, if a program thinks it can detect an invalid seekdir by checking whether errno was set or not by readdir. This is purely hypothetical though. Laurie -- Personal http://tratt.net/laurie/ The Converge programming language http://convergepl.org/ https://github.com/ltratt http://twitter.com/laurencetratt
Re: pfsync: deferred packets exit on wrong interface
On 2011-12-12 08:13, Peter Hallin wrote: Hello, We have a bunch of bridged firewalls and we are now looking into using the pfsync defer feature to solve some problems with async states during failover. However I discovered that the deferred packets (tcp SYN for example) are being sent out on the management interface of the firewall an not on the bridged vlan interfaces where they're supposed to go. In this case the traffic that goes the wrong way is destined for the firewall management network, but as it's only the SYN packet that goes that way, the firewall proctecting the management network will not set a state and drop subsequent packets. It probably takes this way because it's the shortest path to the other firewalls. If the defer flag is off, everything works as it should and traffic takes the right path. This has been tested on 4.9/amd64. Please let me know if I can supply more info, it's a pretty complex problem to explain. Best regards, Peter Hallin Lund University OK, it is my understanding that the defer feature doesn't work with pure bridges is due to the fact that the information on which bridge a deferred packet came from isn't stored. Does anyone have any ideas on how to implement this feature in a bridged setup? The thing that happens now is that a deferred packet coming from a bridge will be sent out on the only interface with an IP address, in our case the management if. I think pfsync defer is a killer feature and I really hope it will work for us some day. Thanks, //Peter
Re: readdir man page
On 3 February 2012 02:50, Philip Guenther guent...@gmail.com wrote:
On Thu, Feb 2, 2012 at 5:29 AM, Christiano F. Haesbaert
haesba...@haesbaert.org wrote:
On 2 February 2012 10:13, Laurence Tratt lau...@tratt.net wrote:
To my surprise (and a couple of hours debugging later), readdir does not
necessarily set errno even if NULL is returned. This is rather confusing
because if NULL is returned two things might have happened:
1) The end of the directory has been reached in a normal fashion; errno
won't have been touched in any way.
2) One of the errors associated with getdirentries might have occurred,
in which case errno will have been set to a specific value.
So any user who writes:
if (readdir(dirp) == NULL) {
if (errno == ...)
...
}
without setting errno to 0 beforehand will get a nasty shock from time to
time (in my case, I was finding EAGAIN was leaking through from a failed
flock call).
Yep. This is how readdir() has always behaved and was standardized by
POSIX.
The current man page doesn't claim that errno is always set but one might
reasonably assume that it is: certainly, it seems a horribly easy way of
introducing bugs (at least for idiots such as myself). Similar issues
(e.g.
I beg to disagree on this, yes, the interface is horrible:
The readdir() function returns a pointer to the next directory entry in
the named directory stream dirp. It returns NULL upon reaching the
end
of the directory or detecting an invalid seekdir() operation.
So it says nothing about errno, but then, in the end:
The readdir() and readdir_r() functions may also fail and set errno for
any of the errors specified for the routine getdirentries(2).
Actually the reasonable thing to me is errno can't be trusted from
readdir(), since may is not good enough.
And if that is the case, errno may actually be set to anything related
to internals.
The use of 'may' is a standard form for the ERRORS section of
manpages. What you may be missing is that the 'may' applies to the
entire second half of the sentence as a whole. The precedence is
similar to:
The readdir() function may also ( fail and set errno ... ).
Yes, you nailed, I didn't get that if readdir() fails it always set errno.
I.e., if it fails, it sets errno.
Although I like your intention, I think the problem is in the later part:
The readdir() and readdir_r() functions may also fail and set errno for
any of the errors specified for the routine getdirentries(2).
Maybe this could be clarified ?
Lets see what others think.
I'll propose a third option: I think the directory(3) manpage would be
clearer if the DESCRIPTION section was unwoven to have separate
DESCRIPTION and RETURN VALUES section. Explicit wording that errno is
not set when end of directory is reached and that programs that want
to tell apart end-of-directory and errors must set errno to zero
should be added.
Agreed
I also think readdir() should set errno if it detects an invalid
seekdir(). EINVAL seems correct.
Philip Guenther
Re: readdir man page
On 3 February 2012 03:29, Philip Guenther guent...@gmail.com wrote:
On Thu, 2 Feb 2012, Philip Guenther wrote:
I also think readdir() should set errno if it detects an invalid
seekdir(). EINVAL seems correct.
Here's a diff for this bit.
oks?
Philip Guenther
Index: gen/readdir.c
===
RCS file: /cvs/src/lib/libc/gen/readdir.c,v
retrieving revision 1.15
diff -u -p -r1.15 readdir.c
--- gen/readdir.c 18 Nov 2009 07:43:22 - 1.15
+++ gen/readdir.c 3 Feb 2012 05:21:58 -
@@ -29,6 +29,7 @@
*/
#include dirent.h
+#include errno.h
#include thread_private.h
/*
@@ -52,12 +53,14 @@ _readdir_unlocked(DIR *dirp, struct dire
return (-1);
}
dp = (struct dirent *)(dirp-dd_buf + dirp-dd_loc);
- if ((long)dp 03) /* bogus pointer check */
- return (-1);
- if (dp-d_reclen = 0 ||
- dp-d_reclen dirp-dd_len + 1 - dirp-dd_loc)
+ if ((long)dp 03 ||/* bogus pointer check */
+ dp-d_reclen = 0 ||
+ dp-d_reclen dirp-dd_len + 1 - dirp-dd_loc) {
+ errno = EINVAL;
return (-1);
+ }
dirp-dd_loc += dp-d_reclen;
+
/*
* When called from seekdir(), we let it decide on
* the end condition to avoid overshooting: the next
ok by me, the other case is assured by the above getdirentries() call.
Re: NEW: libc getdelim(3) and getline(3)
On Fri, Feb 03, 2012 at 05:00:55PM +0100, Jan Klemkow wrote: I decide to remove the check for the negative delimiter character, but I am not sure, with the file handle pointer check!? In my opinion this should be checked and return an minus 1 with an error for invalid input. The realloc(2) memory leak is fixed. The diff for renaming existing geline() functions in base is still work in process. Maybe there are also problems with the ports tree. How could I check this without building the hole ports tree? Most ports use autohell to check for getline() and friends. Those that don't and need these functions are usually patched, so it's easy to find them. In any way, ports should not prevent from getting this in when it's ready and _after_ unlock, we will fix what needs be in ports. The current state of the diff attached for testing. thanks, Jan Index: Makefile.inc === RCS file: /cvs/src/lib/libc/stdio/Makefile.inc,v retrieving revision 1.21 diff -u -p -r1.21 Makefile.inc --- Makefile.inc 18 Jan 2012 14:01:38 - 1.21 +++ Makefile.inc 3 Feb 2012 15:47:43 - @@ -18,12 +18,14 @@ SRCS+=asprintf.c clrerr.c fclose.c fdop fgetwc.c fgetws.c fputwc.c fputws.c fwide.c getwc.c getwchar.c \ putwc.c putwchar.c ungetwc.c \ fwprintf.c swprintf.c vfwprintf.c vswprintf.c vwprintf.c wprintf.c \ - fwscanf.c swscanf.c vfwscanf.c vswscanf.c vwscanf.c wscanf.c + fwscanf.c swscanf.c vfwscanf.c vswscanf.c vwscanf.c wscanf.c \ + getdelim.c MAN+=fclose.3 ferror.3 fflush.3 fgetln.3 fgets.3 fopen.3 fputs.3 \ fread.3 fseek.3 funopen.3 getc.3 mktemp.3 perror.3 printf.3 putc.3 \ remove.3 scanf.3 setbuf.3 stdio.3 tmpnam.3 ungetc.3 \ - fgetws.3 fputws.3 fwide.3 getwc.3 putwc.3 ungetwc.3 wprintf.3 wscanf.3 + fgetws.3 fputws.3 fwide.3 getwc.3 putwc.3 ungetwc.3 wprintf.3 wscanf.3 \ + getdelim.3 MLINKS+=ferror.3 clearerr.3 ferror.3 feof.3 ferror.3 fileno.3 MLINKS+=fflush.3 fpurge.3 Index: getdelim.3 === RCS file: getdelim.3 diff -N getdelim.3 --- /dev/null 1 Jan 1970 00:00:00 - +++ getdelim.33 Feb 2012 15:47:43 - @@ -0,0 +1,156 @@ +.\ $OpenBSD$ +.\ $NetBSD: getdelim.3,v 1.9 2011/04/20 23:37:51 enami Exp $ +.\ +.\ Copyright (c) 2009 The NetBSD Foundation, Inc. +.\ All rights reserved. +.\ +.\ This code is derived from software contributed to The NetBSD Foundation +.\ by Roy Marples. +.\ +.\ 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 NETBSD FOUNDATION, INC. AND CONTRIBUTORS +.\ ``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 FOUNDATION OR CONTRIBUTORS +.\ 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$ +.Dt GETDELIM 3 +.Os +.Sh NAME +.Nm getdelim , +.Nm getline +.Nd read a delimited record from a stream +.Sh LIBRARY +.Lb libc +.Sh SYNOPSIS +.In stdio.h +.Ft ssize_t +.Fn getdelim char ** restrict lineptr size_t * restrict n int delimiter FILE * restrict stream +.Ft ssize_t +.Fn getline char ** restrict lineptr size_t * restrict n FILE * restrict stream +.Sh DESCRIPTION +The +.Fn getdelim +function reads from the +.Fa stream +until it encounters a character matching +.Fa delimiter , +storing the input in +.Fa *lineptr . +The buffer is +.Dv NUL Ns No -terminated +and includes the delimiter. +The +.Fa delimiter +character must be representable as an unsigned char. +.Pp +If +.Fa *n +is non-zero, then +.Fa *lineptr +must be pre-allocated to at least +.Fa *n +bytes. +The buffer should be allocated dynamically; +it must be possible to +.Xr free 3 +.Fa *lineptr . +.Fn getdelim +ensures that +.Fa *lineptr +is large enough to hold the input, updating +.Fa *n +to reflect the
Re: PATCH: failed spl lock release in pmap
What is about this patch?
thanks,
Jan
On Fri, Dec 02, 2011 at 06:31:47PM +0100, Jan Klemkow wrote:
Hello,
I've found an other spl lock lease failure.
It's in an architecture which i couldn't test.
But, I think the diff should work.
bye,
Jan
Index: pmap.c
===
RCS file: /mount/cvsdev/cvs/openbsd/src/sys/arch/solbourne/solbourne/pmap.c,v
retrieving revision 1.5
diff -u -w -p -r1.5 pmap.c
--- pmap.c 30 May 2011 22:25:22 - 1.5
+++ pmap.c 2 Dec 2011 15:46:08 -
@@ -915,9 +915,10 @@ pmap_enter(struct pmap *pmap, vaddr_t va
if ((pte = pmap_grow_pte(pmap, va)) == NULL) {
DPRINTF(PDB_ENTER, ( - pmap_grow_pte failed\n));
- if (flags PMAP_CANFAIL)
+ if (flags PMAP_CANFAIL) {
+ splx(s);
return (ENOMEM);
- else
+ } else
panic(pmap_enter: unable to allocate PT);
}
@@ -974,9 +975,10 @@ pmap_enter(struct pmap *pmap, vaddr_t va
if (cur == NULL) {
cur = pool_get(pvpool, PR_NOWAIT);
if (cur == NULL) {
- if (flags PMAP_CANFAIL)
+ if (flags PMAP_CANFAIL) {
+ splx(s);
return (ENOMEM);
- else
+ } else
panic(pmap_enter:
pvlist pool exhausted);
}
@@ -1210,7 +1212,7 @@ pg_flushcache(struct vm_page *pg)
pvl = pg_to_pvl(pg);
if (pvl-pv_pmap == NULL)
- return;
+ goto out;
/*
* Since cache_flush_page() causes the whole cache to be flushed,
@@ -1219,6 +1221,7 @@ pg_flushcache(struct vm_page *pg)
/* for (; pvl != NULL; pvl = pvl-pv_next) */
cache_flush_page(pvl-pv_va);
+ out:
splx(s);
}
--
Re: PATCH: failed spl lock release in pmap
What is about this patch? This code is currently being rewritten. Miod
PCI_PWR_D[0-3] vs. PCI_PMCSR_STATE_D[0-3]
hello tech@ aren't these PCI_PWR_D[0-3] defines just duplicates of PCI_PMCSR_STATE_D[0-3] from pcireg.h? --- /usr/src/sys/dev/pci/pcivar.h.orig Sat Feb 4 01:28:05 2012 +++ /usr/src/sys/dev/pci/pcivar.h Sat Feb 4 01:28:41 2012 @@ -53,14 +53,6 @@ */ typedef u_int32_t pcireg_t;/* configuration space register XXX */ -/* - * Power Management (PCI 2.2) - */ -#define PCI_PWR_D0 0 -#define PCI_PWR_D1 1 -#define PCI_PWR_D2 2 -#define PCI_PWR_D3 3 - #ifdef _KERNEL struct pcibus_attach_args; --- sys/dev/pci/if_bge.c.orig Sat Feb 4 01:29:50 2012 +++ sys/dev/pci/if_bge.cSat Feb 4 01:32:49 2012 @@ -1851,8 +1851,9 @@ * so force device into D0 state before starting initialization. */ pm_ctl = pci_conf_read(pc, pa-pa_tag, BGE_PCI_PWRMGMT_CMD); - pm_ctl = ~(PCI_PWR_D0|PCI_PWR_D1|PCI_PWR_D2|PCI_PWR_D3); - pm_ctl |= (1 8) | PCI_PWR_D0 ; /* D0 state */ + pm_ctl = ~(PCI_PMCSR_STATE_D0|PCI_PMCSR_STATE_D1| + PCI_PMCSR_STATE_D2|PCI_PMCSR_STATE_D3); + pm_ctl |= (1 8) | PCI_PMCSR_STATE_D0; /* D0 state */ pci_conf_write(pc, pa-pa_tag, BGE_PCI_PWRMGMT_CMD, pm_ctl); DELAY(1000);/* 27 usec is allegedly sufficent */ --- sys/dev/cardbus/if_ath_cardbus.c.orig Sat Feb 4 01:36:00 2012 +++ sys/dev/cardbus/if_ath_cardbus.cSat Feb 4 01:37:04 2012 @@ -282,7 +282,7 @@ #ifdef notyet (void)cardbus_setpowerstate(sc-sc_dev.dv_xname, ct, csc-sc_tag, - PCI_PWR_D0); + PCI_PMCSR_STATE_D0); #endif /* Program the BAR. */ --- sys/dev/cardbus/if_atw_cardbus.c.orig Sat Feb 4 01:37:23 2012 +++ sys/dev/cardbus/if_atw_cardbus.cSat Feb 4 01:38:07 2012 @@ -328,7 +328,7 @@ #ifdef notyet (void)cardbus_setpowerstate(sc-sc_dev.dv_xname, ct, csc-sc_tag, - PCI_PWR_D0); + PCI_PMCSR_STATE_D0); #endif /* Program the BAR. */
sysv_msg queue id error
Index: sys/kern/sysv_msg.c
===
RCS file: /cvs/src/sys/kern/sysv_msg.c,v
retrieving revision 1.24
diff -u -r1.24 sysv_msg.c
--- sys/kern/sysv_msg.c20 May 2011 16:06:25 -1.24
+++ sys/kern/sysv_msg.c3 Feb 2012 23:58:45 -
@@ -230,7 +230,7 @@
goto again;
found:
-*retval = que-que_id;
+*retval = IXSEQ_TO_IPCID(0, que-msqid_ds.msg_perm);
return (error);
}
@@ -421,7 +421,7 @@
struct que *que;
TAILQ_FOREACH(que, msg_queues, que_next)
-if (que-que_id == id)
+if (que-msqid_ds.msg_perm.seq == IPCID_TO_SEQ(id))
break;
/* don't return queues marked for removal */
Index: sys/sys/msg.h
===
RCS file: /cvs/src/sys/sys/msg.h,v
retrieving revision 1.16
diff -u -r1.16 msg.h
--- sys/sys/msg.h3 Jan 2011 23:08:07 -1.16
+++ sys/sys/msg.h3 Feb 2012 23:58:45 -
@@ -62,7 +62,6 @@
struct que {
struct msqid_dsmsqid_ds;
-intque_id;
intque_flags;
intque_references;
que_id was read to return msgq ids and to search for active msgqs but
was never set. msgqid_ds.msg_perm.seq was set but was only read by
user space programs.
Result: only 1 msg queue could be used because queue id 0 was the
only one returned and 0 matched the first queue.
Note: while 32767 semaphore sets, message queues, and SYSV shared memory
segments are probably enough for 5 or 10 years, the use of 0x7FFF, etc.
as masks is ugly. The sysv code is slightly inconsistent in that some
use an array for state and some use tailq. RBtree could be used instead.
I'd be glad to change this but I'm really more concerned with getting
very high performance for an application that's due for a demo next week.
