Re: sysctl machdep.lidaction=suspend

[Martin Natano]

On Sat, Jul 29, 2017 at 02:19:50PM +0200, Martin Natano wrote:
> On Sat, Jul 29, 2017 at 02:03:22PM +0200, Mark Kettenis wrote:
> > 
> > I don't think we want to add string parsing like this in the kernel.
> > Maybe the sysctl(8) frontend should do the mapping from strings to
> > numbers?
> 
> Ok, I'll try to come up with an alternative diff that does the parsing
> in sysctl(8). Let me fetch my rubber gloves...

Here's the alternative diff. Ok?


Index: etc/etc.amd64/sysctl.conf
===
RCS file: /cvs/src/etc/etc.amd64/sysctl.conf,v
retrieving revision 1.7
diff -u -p -r1.7 sysctl.conf
--- etc/etc.amd64/sysctl.conf   2 Mar 2017 10:38:09 -   1.7
+++ etc/etc.amd64/sysctl.conf   25 Jul 2017 18:40:31 -
@@ -1,3 +1,3 @@
 #machdep.allowaperture=2   # See xf86(4)
 #machdep.kbdreset=1# permit console CTRL-ALT-DEL to do a nice halt
-#machdep.lidaction=0   # 1=suspend, 2=hibernate laptop upon lid closing
+#machdep.lidaction=none# action upon lid closing: none, 
suspend or hibernate
Index: etc/etc.i386/sysctl.conf
===
RCS file: /cvs/src/etc/etc.i386/sysctl.conf,v
retrieving revision 1.21
diff -u -p -r1.21 sysctl.conf
--- etc/etc.i386/sysctl.conf2 Mar 2017 10:38:09 -   1.21
+++ etc/etc.i386/sysctl.conf25 Jul 2017 18:40:35 -
@@ -1,4 +1,4 @@
 #machdep.allowaperture=2   # See xf86(4)
 #machdep.apmhalt=1 # 1=powerdown hack, try if halt -p doesn't work
 #machdep.kbdreset=1# permit console CTRL-ALT-DEL to do a nice halt
-#machdep.lidaction=0   # 1=suspend, 2=hibernate laptop upon lid closing
+#machdep.lidaction=none# action upon lid closing: none, 
suspend or hibernate
Index: etc/etc.loongson/sysctl.conf
===
RCS file: /cvs/src/etc/etc.loongson/sysctl.conf,v
retrieving revision 1.4
diff -u -p -r1.4 sysctl.conf
--- etc/etc.loongson/sysctl.conf2 Mar 2017 10:38:09 -   1.4
+++ etc/etc.loongson/sysctl.conf25 Jul 2017 18:40:40 -
@@ -1 +1 @@
-#machdep.lidaction=0   # 1=suspend, 2=hibernate laptop upon lid closing
+#machdep.lidaction=none# action upon lid closing: none, 
suspend or hibernate
Index: sbin/sysctl/sysctl.c
===
RCS file: /cvs/src/sbin/sysctl/sysctl.c,v
retrieving revision 1.228
diff -u -p -r1.228 sysctl.c
--- sbin/sysctl/sysctl.c19 Jul 2017 06:30:54 -  1.228
+++ sbin/sysctl/sysctl.c29 Jul 2017 16:57:13 -
@@ -158,6 +158,15 @@ struct list secondlevel[] = {
{ 0, 0 },   /* CTL_VFS */
 };
 
+struct enum_vals {
+   int  size;
+   const char  **names;
+};
+#ifdef CPU_LIDACTION
+const char *cpu_lidaction_names[] = { "none", "suspend", "hibernate" };
+struct enum_vals cpu_lidaction_enum = { 3, cpu_lidaction_names };
+#endif
+
 intAflag, aflag, nflag, qflag;
 
 /*
@@ -167,10 +176,10 @@ int   Aflag, aflag, nflag, qflag;
 #defineBOOTTIME0x0002
 #defineCHRDEV  0x0004
 #defineBLKDEV  0x0008
+#define ENUM   0x0010
 #defineBADDYNAMIC  0x0020
 #defineBIOSGEO 0x0040
 #defineBIOSDEV 0x0080
-#defineMAJ2DEV 0x0100
 #defineUNSIGNED0x0200
 #defineKMEMBUCKETS 0x0400
 #defineLONGARRAY   0x0800
@@ -211,6 +220,8 @@ void print_sensor(struct sensor *);
 int sysctl_chipset(char *, char **, int *, int, int *);
 #endif
 void vfsinit(void);
+int strtoenum(const char *, struct enum_vals *, const char **);
+const char *enumtostr(int, struct enum_vals *);
 
 char *equ = "=";
 
@@ -297,6 +308,7 @@ parse(char *string, int flags)
int indx, type, state, intval, len;
size_t size, newsize = 0;
int lal = 0, special = 0;
+   struct enum_vals *enump;
void *newval = NULL;
int64_t quadval;
struct list *lp;
@@ -615,6 +627,12 @@ parse(char *string, int flags)
if (mib[1] == CPU_CPUFEATURE)
special |= HEX;
 #endif
+#ifdef CPU_LIDACTION
+   if (mib[1] == CPU_LIDACTION) {
+   special |= ENUM;
+   enump = _lidaction_enum;
+   }
+#endif
 #ifdef CPU_BLK2CHR
if (mib[1] == CPU_BLK2CHR) {
if (bufp == NULL)
@@ -700,6 +718,8 @@ parse(char *string, int flags)
case CTLTYPE_INT:
if (special & UNSIGNED)
intval = strtonum(newval, 0, UINT_MAX, );
+   else if (special & ENUM)
+

Re: sysctl machdep.lidaction=suspend

[Martin Natano]

On Sat, Jul 29, 2017 at 02:03:22PM +0200, Mark Kettenis wrote:
> 
> I don't think we want to add string parsing like this in the kernel.
> Maybe the sysctl(8) frontend should do the mapping from strings to
> numbers?

Ok, I'll try to come up with an alternative diff that does the parsing
in sysctl(8). Let me fetch my rubber gloves...

When the necessary mechanism is there we can also take advantage of it
for setting hw.perfpolicy. That's where I copied the approach from.

> 
> > Index: etc/etc.amd64/sysctl.conf
> > ===
> > RCS file: /cvs/src/etc/etc.amd64/sysctl.conf,v
> > retrieving revision 1.7
> > diff -u -p -r1.7 sysctl.conf
> > --- etc/etc.amd64/sysctl.conf   2 Mar 2017 10:38:09 -   1.7
> > +++ etc/etc.amd64/sysctl.conf   25 Jul 2017 18:40:31 -
> > @@ -1,3 +1,3 @@
> >  #machdep.allowaperture=2   # See xf86(4)
> >  #machdep.kbdreset=1# permit console CTRL-ALT-DEL to do a 
> > nice halt
> > -#machdep.lidaction=0   # 1=suspend, 2=hibernate laptop upon 
> > lid closing
> > +#machdep.lidaction=none# action upon lid closing: none, 
> > suspend or hibernate
> > Index: etc/etc.i386/sysctl.conf
> > ===
> > RCS file: /cvs/src/etc/etc.i386/sysctl.conf,v
> > retrieving revision 1.21
> > diff -u -p -r1.21 sysctl.conf
> > --- etc/etc.i386/sysctl.conf2 Mar 2017 10:38:09 -   1.21
> > +++ etc/etc.i386/sysctl.conf25 Jul 2017 18:40:35 -
> > @@ -1,4 +1,4 @@
> >  #machdep.allowaperture=2   # See xf86(4)
> >  #machdep.apmhalt=1 # 1=powerdown hack, try if halt -p doesn't work
> >  #machdep.kbdreset=1# permit console CTRL-ALT-DEL to do a 
> > nice halt
> > -#machdep.lidaction=0   # 1=suspend, 2=hibernate laptop upon 
> > lid closing
> > +#machdep.lidaction=none# action upon lid closing: none, 
> > suspend or hibernate
> > Index: etc/etc.loongson/sysctl.conf
> > ===
> > RCS file: /cvs/src/etc/etc.loongson/sysctl.conf,v
> > retrieving revision 1.4
> > diff -u -p -r1.4 sysctl.conf
> > --- etc/etc.loongson/sysctl.conf2 Mar 2017 10:38:09 -   1.4
> > +++ etc/etc.loongson/sysctl.conf25 Jul 2017 18:40:40 -
> > @@ -1 +1 @@
> > -#machdep.lidaction=0   # 1=suspend, 2=hibernate laptop upon 
> > lid closing
> > +#machdep.lidaction=none# action upon lid closing: none, 
> > suspend or hibernate
> > Index: sys/arch/amd64/amd64/machdep.c
> > ===
> > RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v
> > retrieving revision 1.231
> > diff -u -p -r1.231 machdep.c
> > --- sys/arch/amd64/amd64/machdep.c  12 Jul 2017 06:26:32 -  1.231
> > +++ sys/arch/amd64/amd64/machdep.c  25 Jul 2017 19:37:22 -
> > @@ -264,6 +264,8 @@ voidmap_tramps(void);
> >  void   init_x86_64(paddr_t);
> >  void   (*cpuresetfn)(void);
> >  
> > +intsysctl_cpulidaction(void *, size_t *, void *, size_t);
> > +
> >  #ifdef APERTURE
> >  int allowaperture = 0;
> >  #endif
> > @@ -428,7 +430,7 @@ cpu_sysctl(int *name, u_int namelen, voi
> > extern int amd64_has_xcrypt;
> > dev_t consdev;
> > dev_t dev;
> > -   int val, error;
> > +   int error;
> >  
> > switch (name[0]) {
> > case CPU_CONSDEV:
> > @@ -477,15 +479,7 @@ cpu_sysctl(int *name, u_int namelen, voi
> > case CPU_XCRYPT:
> > return (sysctl_rdint(oldp, oldlenp, newp, amd64_has_xcrypt));
> > case CPU_LIDACTION:
> > -   val = lid_action;
> > -   error = sysctl_int(oldp, oldlenp, newp, newlen, );
> > -   if (!error) {
> > -   if (val < 0 || val > 2)
> > -   error = EINVAL;
> > -   else
> > -   lid_action = val;
> > -   }
> > -   return (error);
> > +   return (sysctl_cpulidaction(oldp, oldlenp, newp, newlen));
> >  #if NPCKBC > 0 && NUKBD > 0
> > case CPU_FORCEUKBD:
> > if (forceukbd)
> > @@ -500,6 +494,47 @@ cpu_sysctl(int *name, u_int namelen, voi
> > return (EOPNOTSUPP);
> > }
> > /* NOTREACHED */
> > +}
> > +
> > +int
> > +sysctl_cpulidaction(void *oldp, size_t *oldlenp, void *newp, size_t newlen)
> > +{
> > +   char action[10];
> > +   int error;
> > +
> > +   switch (lid_action) {
> > +   case 0:
> > +   default:
> > +   strlcpy(action, "none", sizeof(action));
> > +   break;
> > +   case 1:
> > +   strlcpy(action, "suspend", sizeof(action));
> > +   break;
> > +#ifdef HIBERNATE
> > +   case 2:
> > +   strlcpy(action, "hibernate", sizeof(action));
> > +   break;
> > +#endif
> > +   }
> > +
> > +   error = sysctl_string(oldp, oldlenp, newp, newlen, action, 
> > sizeof(action));
> > +   if (error)
> > +  

sysctl machdep.lidaction=suspend

[Martin Natano]

Words are more descriptive than numbers. Let's use them!

This diff removes

sysctl machdep.lidaction=0
sysctl machdep.lidaction=1
sysctl machdep.lidaction=2

in favor of the more descriptive

sysctl machdep.lidaction=none
sysctl machdep.lidaction=suspend
sysctl machdep.lidaction=hibernate

as requested by deraadt.

Given that there is a diff for poweroff (which I want to see go in)
floating around on tech@, it might be a good idea to switch now, so
the numbering doesn't get out of hand.

Of course this means people will have to update their /etc/sysctl.conf
_again_. Sorry for that, I missed the opportunity to do both lidaction
changes in one big swoop.

Do we want to go there?

natano


Index: etc/etc.amd64/sysctl.conf
===
RCS file: /cvs/src/etc/etc.amd64/sysctl.conf,v
retrieving revision 1.7
diff -u -p -r1.7 sysctl.conf
--- etc/etc.amd64/sysctl.conf   2 Mar 2017 10:38:09 -   1.7
+++ etc/etc.amd64/sysctl.conf   25 Jul 2017 18:40:31 -
@@ -1,3 +1,3 @@
 #machdep.allowaperture=2   # See xf86(4)
 #machdep.kbdreset=1# permit console CTRL-ALT-DEL to do a nice halt
-#machdep.lidaction=0   # 1=suspend, 2=hibernate laptop upon lid closing
+#machdep.lidaction=none# action upon lid closing: none, 
suspend or hibernate
Index: etc/etc.i386/sysctl.conf
===
RCS file: /cvs/src/etc/etc.i386/sysctl.conf,v
retrieving revision 1.21
diff -u -p -r1.21 sysctl.conf
--- etc/etc.i386/sysctl.conf2 Mar 2017 10:38:09 -   1.21
+++ etc/etc.i386/sysctl.conf25 Jul 2017 18:40:35 -
@@ -1,4 +1,4 @@
 #machdep.allowaperture=2   # See xf86(4)
 #machdep.apmhalt=1 # 1=powerdown hack, try if halt -p doesn't work
 #machdep.kbdreset=1# permit console CTRL-ALT-DEL to do a nice halt
-#machdep.lidaction=0   # 1=suspend, 2=hibernate laptop upon lid closing
+#machdep.lidaction=none# action upon lid closing: none, 
suspend or hibernate
Index: etc/etc.loongson/sysctl.conf
===
RCS file: /cvs/src/etc/etc.loongson/sysctl.conf,v
retrieving revision 1.4
diff -u -p -r1.4 sysctl.conf
--- etc/etc.loongson/sysctl.conf2 Mar 2017 10:38:09 -   1.4
+++ etc/etc.loongson/sysctl.conf25 Jul 2017 18:40:40 -
@@ -1 +1 @@
-#machdep.lidaction=0   # 1=suspend, 2=hibernate laptop upon lid closing
+#machdep.lidaction=none# action upon lid closing: none, 
suspend or hibernate
Index: sys/arch/amd64/amd64/machdep.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v
retrieving revision 1.231
diff -u -p -r1.231 machdep.c
--- sys/arch/amd64/amd64/machdep.c  12 Jul 2017 06:26:32 -  1.231
+++ sys/arch/amd64/amd64/machdep.c  25 Jul 2017 19:37:22 -
@@ -264,6 +264,8 @@ voidmap_tramps(void);
 void   init_x86_64(paddr_t);
 void   (*cpuresetfn)(void);
 
+intsysctl_cpulidaction(void *, size_t *, void *, size_t);
+
 #ifdef APERTURE
 int allowaperture = 0;
 #endif
@@ -428,7 +430,7 @@ cpu_sysctl(int *name, u_int namelen, voi
extern int amd64_has_xcrypt;
dev_t consdev;
dev_t dev;
-   int val, error;
+   int error;
 
switch (name[0]) {
case CPU_CONSDEV:
@@ -477,15 +479,7 @@ cpu_sysctl(int *name, u_int namelen, voi
case CPU_XCRYPT:
return (sysctl_rdint(oldp, oldlenp, newp, amd64_has_xcrypt));
case CPU_LIDACTION:
-   val = lid_action;
-   error = sysctl_int(oldp, oldlenp, newp, newlen, );
-   if (!error) {
-   if (val < 0 || val > 2)
-   error = EINVAL;
-   else
-   lid_action = val;
-   }
-   return (error);
+   return (sysctl_cpulidaction(oldp, oldlenp, newp, newlen));
 #if NPCKBC > 0 && NUKBD > 0
case CPU_FORCEUKBD:
if (forceukbd)
@@ -500,6 +494,47 @@ cpu_sysctl(int *name, u_int namelen, voi
return (EOPNOTSUPP);
}
/* NOTREACHED */
+}
+
+int
+sysctl_cpulidaction(void *oldp, size_t *oldlenp, void *newp, size_t newlen)
+{
+   char action[10];
+   int error;
+
+   switch (lid_action) {
+   case 0:
+   default:
+   strlcpy(action, "none", sizeof(action));
+   break;
+   case 1:
+   strlcpy(action, "suspend", sizeof(action));
+   break;
+#ifdef HIBERNATE
+   case 2:
+   strlcpy(action, "hibernate", sizeof(action));
+   break;
+#endif
+   }
+
+   error = sysctl_string(oldp, oldlenp, newp, newlen, action, 
sizeof(action));
+   if (error)
+   return error;
+
+   if (newp != 

Re: remove CPU_LIDSUSPEND/machdep.lidsuspend

[Martin Natano]

On Tue, Jul 11, 2017 at 02:40:16PM -0700, Mike Larkin wrote:
> On Tue, Jul 11, 2017 at 11:19:17PM +0200, Martin Natano wrote:
> > Go ahead replacing machdep.lidsuspend with cpu.lidaction. OpenBSD 6.1
> 
> cpu.lidaction or machdep.lidaction?

machdep.lidaction; I got confused by the #define name.

remove CPU_LIDSUSPEND/machdep.lidsuspend

[Martin Natano]

Go ahead replacing machdep.lidsuspend with cpu.lidaction. OpenBSD 6.1
contains both MIBs, so there is a clear path for migration.

ok?

natano


Index: arch/amd64/amd64/machdep.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v
retrieving revision 1.230
diff -u -p -r1.230 machdep.c
--- arch/amd64/amd64/machdep.c  29 Jun 2017 07:19:40 -  1.230
+++ arch/amd64/amd64/machdep.c  11 Jul 2017 20:44:44 -
@@ -476,7 +476,6 @@ cpu_sysctl(int *name, u_int namelen, voi
 #endif
case CPU_XCRYPT:
return (sysctl_rdint(oldp, oldlenp, newp, amd64_has_xcrypt));
-   case CPU_LIDSUSPEND:
case CPU_LIDACTION:
val = lid_action;
error = sysctl_int(oldp, oldlenp, newp, newlen, );
Index: arch/amd64/include/cpu.h
===
RCS file: /cvs/src/sys/arch/amd64/include/cpu.h,v
retrieving revision 1.112
diff -u -p -r1.112 cpu.h
--- arch/amd64/include/cpu.h20 Jun 2017 05:34:41 -  1.112
+++ arch/amd64/include/cpu.h11 Jul 2017 20:47:07 -
@@ -427,7 +427,6 @@ void mp_setperf_init(void);
 #define CPU_KBDRESET   10  /* keyboard reset under pcvt */
 #define CPU_APMHALT11  /* halt -p hack */
 #define CPU_XCRYPT 12  /* supports VIA xcrypt in userland */
-#define CPU_LIDSUSPEND 13  /* lid close causes a suspend */
 #define CPU_LIDACTION  14  /* action caused by lid close */
 #define CPU_FORCEUKBD  15  /* Force ukbd(4) as console keyboard */
 #define CPU_MAXID  16  /* number of valid machdep ids */
@@ -446,7 +445,7 @@ void mp_setperf_init(void);
{ "kbdreset", CTLTYPE_INT }, \
{ "apmhalt", CTLTYPE_INT }, \
{ "xcrypt", CTLTYPE_INT }, \
-   { "lidsuspend", CTLTYPE_INT }, \
+   { 0, 0 }, \
{ "lidaction", CTLTYPE_INT }, \
{ "forceukbd", CTLTYPE_INT }, \
 }
Index: arch/arm/include/cpu.h
===
RCS file: /cvs/src/sys/arch/arm/include/cpu.h,v
retrieving revision 1.46
diff -u -p -r1.46 cpu.h
--- arch/arm/include/cpu.h  27 Apr 2017 22:41:46 -  1.46
+++ arch/arm/include/cpu.h  11 Jul 2017 20:48:38 -
@@ -65,7 +65,7 @@
/*  9  formerly int: CPU_ZTSRAWMODE */
/*  10 formerly struct: CPU_ZTSSCALE */
 #defineCPU_MAXSPEED11  /* int: number of valid machdep 
ids */
-#define CPU_LIDSUSPEND 12  /* int: closing lid causes suspend */
+   /*  12 formerly int: CPU_LIDSUSPEND */
 #define CPU_LIDACTION  13  /* action caused by lid close */
 #defineCPU_MAXID   14  /* number of valid machdep ids 
*/
 
@@ -82,7 +82,7 @@
{ 0, 0 }, \
{ 0, 0 }, \
{ "maxspeed", CTLTYPE_INT }, \
-   { "lidsuspend", CTLTYPE_INT }, \
+   { 0, 0 }, \
{ "lidaction", CTLTYPE_INT }, \
 }
 
Index: arch/i386/i386/machdep.c
===
RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v
retrieving revision 1.603
diff -u -p -r1.603 machdep.c
--- arch/i386/i386/machdep.c29 Jun 2017 07:11:24 -  1.603
+++ arch/i386/i386/machdep.c11 Jul 2017 20:50:33 -
@@ -3550,7 +3550,6 @@ cpu_sysctl(int *name, u_int namelen, voi
return (sysctl_rdint(oldp, oldlenp, newp, i386_has_sse2));
case CPU_XCRYPT:
return (sysctl_rdint(oldp, oldlenp, newp, i386_has_xcrypt));
-   case CPU_LIDSUSPEND:
case CPU_LIDACTION:
val = lid_action;
error = sysctl_int(oldp, oldlenp, newp, newlen, );
Index: arch/i386/include/cpu.h
===
RCS file: /cvs/src/sys/arch/i386/include/cpu.h,v
retrieving revision 1.155
diff -u -p -r1.155 cpu.h
--- arch/i386/include/cpu.h 16 Mar 2017 10:02:03 -  1.155
+++ arch/i386/include/cpu.h 11 Jul 2017 20:46:11 -
@@ -523,7 +523,6 @@ int cpu_paenable(void *);
 #define CPU_SSE14  /* supports SSE */
 #define CPU_SSE2   15  /* supports SSE2 */
 #define CPU_XCRYPT 16  /* supports VIA xcrypt in userland */
-#define CPU_LIDSUSPEND 17  /* lid close causes a suspend */
 #define CPU_LIDACTION  18  /* action caused by lid close */
 #define CPU_FORCEUKBD  19  /* Force ukbd(4) as console keyboard */
 #define CPU_MAXID  20  /* number of valid machdep ids */
@@ -546,7 +545,7 @@ int cpu_paenable(void *);
{ "sse", CTLTYPE_INT }, \
{ "sse2", CTLTYPE_INT }, \
{ "xcrypt", CTLTYPE_INT }, \
-   { "lidsuspend", CTLTYPE_INT }, \
+   { 0, 0 }, \
{ "lidaction", CTLTYPE_INT }, \
{ "forceukbd", CTLTYPE_INT 

Re: vi(1): remove stub settings

[Martin Natano]

On Fri, Jun 30, 2017 at 09:19:31AM -0600, Anthony J. Bentley wrote:
> 
> Here's a diff that updates the STANDARDS section in the manual.
> I think it will be enough to talk about our POSIX compliance. POSIX
> discusses its differences from historical ex/vi in great detail,
> including an explicit recommendation to delete modelines and sourceany.

The diff reads fine to me. OK.


> 
> Index: common/main.c
> ===
> RCS file: /cvs/src/usr.bin/vi/common/main.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 main.c
> --- common/main.c 18 Apr 2017 01:45:35 -  1.39
> +++ common/main.c 30 Jun 2017 15:16:56 -
> @@ -52,7 +52,7 @@ editor(GS *gp, int argc, char *argv[])
>   SCR *sp;
>   size_t len;
>   u_int flags;
> - int ch, flagchk, lflag, secure, startup, readonly, rval, silent;
> + int ch, flagchk, secure, startup, readonly, rval, silent;
>   char *tag_f, *wsizearg, path[256];
>  
>   static const char *optstr[3] = {
> @@ -114,7 +114,7 @@ editor(GS *gp, int argc, char *argv[])
>   /* Parse the arguments. */
>   flagchk = '\0';
>   tag_f = wsizearg = NULL;
> - lflag = secure = silent = 0;
> + secure = silent = 0;
>   startup = 1;
>  
>   /* Set the file snapshot flag. */
> @@ -163,9 +163,6 @@ editor(GS *gp, int argc, char *argv[])
>   case 'F':   /* No snapshot. */
>   F_CLR(gp, G_SNAPSHOT);
>   break;
> - case 'l':   /* Set lisp, showmatch options. */
> - lflag = 1;
> - break;
>   case 'R':   /* Readonly. */
>   readonly = 1;
>   break;
> @@ -260,11 +257,7 @@ editor(GS *gp, int argc, char *argv[])
>   goto err;
>  
>   { int oargs[5], *oargp = oargs;
> - if (lflag) {/* Command-line options. */
> - *oargp++ = O_LISP;
> - *oargp++ = O_SHOWMATCH;
> - }
> - if (readonly)
> + if (readonly)   /* Command-line options. */
>   *oargp++ = O_READONLY;
>   if (secure)
>   *oargp++ = O_SECURE;
> Index: common/options.c
> ===
> RCS file: /cvs/src/usr.bin/vi/common/options.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 options.c
> --- common/options.c  30 Jun 2017 14:42:05 -  1.24
> +++ common/options.c  30 Jun 2017 15:16:56 -
> @@ -89,12 +89,6 @@ OPTLIST const optlist[] = {
>   {"leftright",   f_reformat, OPT_0BOOL,  0},
>  /* O_LINES 4.4BSD */
>   {"lines",   f_lines,OPT_NUM,OPT_NOSAVE},
> -/* O_LISP4BSD
> - *   XXX
> - *   When the lisp option is implemented, delete the OPT_NOSAVE flag,
> - *   so that :mkexrc dumps it.
> - */
> - {"lisp",f_lisp, OPT_0BOOL,  OPT_NOSAVE},
>  /* O_LIST4BSD */
>   {"list",f_reformat, OPT_0BOOL,  0},
>  /* O_LOCKFILES 4.4BSD
> @@ -109,15 +103,6 @@ OPTLIST const optlist[] = {
>   {"matchtime",   NULL,   OPT_NUM,0},
>  /* O_MESG4BSD */
>   {"mesg",NULL,   OPT_1BOOL,  0},
> -/* O_MODELINE4BSD
> - *   !!!
> - *   This has been documented in historical systems as both "modeline"
> - *   and as "modelines".  Regardless of the name, this option represents
> - *   a security problem of mammoth proportions, not to mention a stunning
> - *   example of what your intro CS professor referred to as the perils of
> - *   mixing code and data.  Don't add it, or I will kill you.
> - */
> - {"modeline",NULL,   OPT_0BOOL,  OPT_NOSET},
>  /* O_NOPRINT   4.4BSD */
>   {"noprint", f_print,OPT_STR,OPT_EARLYSET},
>  /* O_NUMBER  4BSD */
> @@ -126,8 +111,6 @@ OPTLIST const optlist[] = {
>   {"octal",   f_print,OPT_0BOOL,  OPT_EARLYSET},
>  /* O_OPEN4BSD */
>   {"open",NULL,   OPT_1BOOL,  0},
> -/* O_OPTIMIZE4BSD */
> - {"optimize",NULL,   OPT_1BOOL,  0},
>  /* O_PARAGRAPHS  4BSD */
>   {"paragraphs",  f_paragraph,OPT_STR,0},
>  /* O_PATH  4.4BSD */
> @@ -140,8 +123,6 @@ OPTLIST const optlist[] = {
>   {"readonly",f_readonly, OPT_0BOOL,  OPT_ALWAYS},
>  /* O_RECDIR4.4BSD */
>   {"recdir",  NULL,   OPT_STR,0},
> -/* O_REDRAW  4BSD */
> - {"redraw",  NULL,   OPT_0BOOL,  0},
>  /* O_REMAP   4BSD */
>   {"remap",   NULL,   OPT_1BOOL,  0},
>  /* O_REPORT  4BSD */
> @@ -168,17 +149,6 @@ OPTLIST const optlist[] = {
>   {"showmode",NULL,   OPT_0BOOL,  0},
>  /* O_SIDESCROLL4.4BSD */
>   {"sidescroll",  NULL,   OPT_NUM,OPT_NOZERO},
> -/* 

Re: CPU_LIDSUSPEND in init(8) and reboot(8)

[Martin Natano]

ping?


On Sun, May 21, 2017 at 09:46:45AM +0200, Martin Natano wrote:
> While switching init and reboot to CPU_LIDACTION, I forgot about the
> #ifdef's. Ok?
> 
> natano
> 
> 
> Index: init/init.c
> ===
> RCS file: /cvs/src/sbin/init/init.c,v
> retrieving revision 1.64
> diff -u -p -r1.64 init.c
> --- init/init.c   3 May 2017 09:51:39 -   1.64
> +++ init/init.c   21 May 2017 07:25:07 -
> @@ -1325,7 +1325,7 @@ f_nice_death(void)
>   static const int death_sigs[3] = { SIGHUP, SIGTERM, SIGKILL };
>   int status;
>  
> -#ifdef CPU_LIDSUSPEND
> +#ifdef CPU_LIDACTION
>   int mib[] = {CTL_MACHDEP, CPU_LIDACTION};
>   int lidaction = 0;
>  
> Index: reboot/reboot.c
> ===
> RCS file: /cvs/src/sbin/reboot/reboot.c,v
> retrieving revision 1.36
> diff -u -p -r1.36 reboot.c
> --- reboot/reboot.c   2 Mar 2017 10:38:09 -   1.36
> +++ reboot/reboot.c   21 May 2017 07:25:31 -
> @@ -112,7 +112,7 @@ main(int argc, char *argv[])
>   if (geteuid())
>   errx(1, "%s", strerror(EPERM));
>  
> -#ifdef CPU_LIDSUSPEND
> +#ifdef CPU_LIDACTION
>   if (howto & RB_POWERDOWN) {
>   /* Disable suspending on laptop lid close */
>   int mib[] = {CTL_MACHDEP, CPU_LIDACTION};
> @@ -122,7 +122,7 @@ main(int argc, char *argv[])
>   sizeof(lidaction)) == -1 && errno != EOPNOTSUPP)
>   warn("sysctl");
>   }
> -#endif /* CPU_LIDSUSPEND */
> +#endif /* CPU_LIDACTION */
>  
>   if (qflag) {
>   reboot(howto);

Re: ping: Style fixes/cleanups

[Martin Natano]

On Tue, Jun 13, 2017 at 08:02:13PM +0200, Klemens Nanni wrote:
> Unify option checking and simply logic.
> 
> F_HDRINCL and F_ROUTE are mutually exclusive, thus check the latter only
> if the former one is not set.
> 
> Index: ping.c
> ===
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.218
> diff -u -p -r1.218 ping.c
> --- ping.c22 Feb 2017 13:43:35 -  1.218
> +++ ping.c13 Jun 2017 17:38:22 -
> @@ -562,17 +562,17 @@ main(int argc, char *argv[])
>   (void)setsockopt(s, SOL_SOCKET, SO_DEBUG, ,
>   sizeof(optval));
> 
> - if ((options & F_FLOOD) && (options & F_INTERVAL))
> + if (options & (F_FLOOD | F_INTERVAL))
>   errx(1, "-f and -i options are incompatible");
> 
> - if ((options & F_FLOOD) && (options & (F_AUD_RECV | F_AUD_MISS)))
> + if (options & (F_FLOOD | F_AUD_RECV | F_AUD_MISS))
>   warnx("No audible output for flood pings");

Did you try to compile and run this?

if (options & (F_FLOOD | F_INTERVAL))

is equivalent to

if ((options & F_FLOOD) || (options & F_INTERVAL))
^^
so this changes behaviour and is most likely wrong.


natano

CPU_LIDSUSPEND in init(8) and reboot(8)

[Martin Natano]

While switching init and reboot to CPU_LIDACTION, I forgot about the
#ifdef's. Ok?

natano


Index: init/init.c
===
RCS file: /cvs/src/sbin/init/init.c,v
retrieving revision 1.64
diff -u -p -r1.64 init.c
--- init/init.c 3 May 2017 09:51:39 -   1.64
+++ init/init.c 21 May 2017 07:25:07 -
@@ -1325,7 +1325,7 @@ f_nice_death(void)
static const int death_sigs[3] = { SIGHUP, SIGTERM, SIGKILL };
int status;
 
-#ifdef CPU_LIDSUSPEND
+#ifdef CPU_LIDACTION
int mib[] = {CTL_MACHDEP, CPU_LIDACTION};
int lidaction = 0;
 
Index: reboot/reboot.c
===
RCS file: /cvs/src/sbin/reboot/reboot.c,v
retrieving revision 1.36
diff -u -p -r1.36 reboot.c
--- reboot/reboot.c 2 Mar 2017 10:38:09 -   1.36
+++ reboot/reboot.c 21 May 2017 07:25:31 -
@@ -112,7 +112,7 @@ main(int argc, char *argv[])
if (geteuid())
errx(1, "%s", strerror(EPERM));
 
-#ifdef CPU_LIDSUSPEND
+#ifdef CPU_LIDACTION
if (howto & RB_POWERDOWN) {
/* Disable suspending on laptop lid close */
int mib[] = {CTL_MACHDEP, CPU_LIDACTION};
@@ -122,7 +122,7 @@ main(int argc, char *argv[])
sizeof(lidaction)) == -1 && errno != EOPNOTSUPP)
warn("sysctl");
}
-#endif /* CPU_LIDSUSPEND */
+#endif /* CPU_LIDACTION */
 
if (qflag) {
reboot(howto);

Re: Fix comment into sys/dev/acpi/acpibtn.c

[Martin Natano]

On Thu, May 11, 2017 at 01:11:16PM +0200, David Coppa wrote:
> 
> I think this comment was copy-pasted as is from the comment some
> lines below, but this is about hibernation, not sleep.

sleep != suspend

Suspend and hibernate both are sleep states.


> 
> Ok?
> 
> Index: acpibtn.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpibtn.c,v
> retrieving revision 1.44
> diff -u -p -u -p -r1.44 acpibtn.c
> --- acpibtn.c 2 Mar 2017 10:38:10 -   1.44
> +++ acpibtn.c 11 May 2017 11:10:21 -
> @@ -236,7 +236,7 @@ acpibtn_notify(struct aml_node *node, in
>   goto sleep;
>  #ifdef HIBERNATE
>   case 2:
> - /* Request to go to sleep */
> + /* Request hibernation */
>   if (acpi_record_event(sc->sc_acpi, 
> APM_USER_HIBERNATE_REQ))
>   acpi_addtask(sc->sc_acpi, acpi_sleep_task,
>   sc->sc_acpi, ACPI_SLEEP_HIBERNATE);
> 

switch base tools from /dev/bpf0 to /dev/bpf

[Martin Natano]

Now that /dev/bpf has been around for two releases, it should be safe to
move the base tools to /dev/bpf. No need to worry about the upgrade path
anymore.

Ok?

natano


Index: lib/libpcap/pcap-bpf.c
===
RCS file: /cvs/src/lib/libpcap/pcap-bpf.c,v
retrieving revision 1.34
diff -u -p -r1.34 pcap-bpf.c
--- lib/libpcap/pcap-bpf.c  8 May 2016 08:20:50 -   1.34
+++ lib/libpcap/pcap-bpf.c  18 Apr 2017 18:29:27 -
@@ -216,9 +216,9 @@ bpf_open(pcap_t *p)
 {
int fd;
 
-   fd = open("/dev/bpf0", O_RDWR);
+   fd = open("/dev/bpf", O_RDWR);
if (fd == -1 && errno == EACCES)
-   fd = open("/dev/bpf0", O_RDONLY);
+   fd = open("/dev/bpf", O_RDONLY);
 
if (fd == -1) {
if (errno == EACCES)
Index: sbin/dhclient/bpf.c
===
RCS file: /cvs/src/sbin/dhclient/bpf.c,v
retrieving revision 1.49
diff -u -p -r1.49 bpf.c
--- sbin/dhclient/bpf.c 18 Apr 2017 13:59:09 -  1.49
+++ sbin/dhclient/bpf.c 18 Apr 2017 18:30:47 -
@@ -78,13 +78,13 @@ if_register_bpf(struct interface_info *i
struct ifreq ifr;
int sock;
 
-   if ((sock = open("/dev/bpf0", O_RDWR | O_CLOEXEC)) == -1)
+   if ((sock = open("/dev/bpf", O_RDWR | O_CLOEXEC)) == -1)
fatal("Can't open bpf");
 
/* Set the BPF device to point at this interface. */
strlcpy(ifr.ifr_name, ifi->name, IFNAMSIZ);
if (ioctl(sock, BIOCSETIF, ) < 0)
-   fatal("Can't attach interface %s to /dev/bpf0", ifi->name);
+   fatal("Can't attach interface %s to /dev/bpf", ifi->name);
 
return (sock);
 }
Index: usr.sbin/arp/arp.c
===
RCS file: /cvs/src/usr.sbin/arp/arp.c,v
retrieving revision 1.78
diff -u -p -r1.78 arp.c
--- usr.sbin/arp/arp.c  15 Apr 2017 11:50:24 -  1.78
+++ usr.sbin/arp/arp.c  18 Apr 2017 18:31:21 -
@@ -821,7 +821,7 @@ wake(const char *ether_addr, const char 
char*pname = NULL;
int  bpf;
 
-   if ((bpf = open("/dev/bpf0", O_RDWR)) == -1)
+   if ((bpf = open("/dev/bpf", O_RDWR)) == -1)
err(1, "Failed to bind to bpf");
 
if (iface == NULL) {
Index: usr.sbin/dhcpd/bpf.c
===
RCS file: /cvs/src/usr.sbin/dhcpd/bpf.c,v
retrieving revision 1.18
diff -u -p -r1.18 bpf.c
--- usr.sbin/dhcpd/bpf.c18 Apr 2017 13:59:09 -  1.18
+++ usr.sbin/dhcpd/bpf.c18 Apr 2017 18:31:37 -
@@ -77,7 +77,7 @@ if_register_bpf(struct interface_info *i
 {
int sock;
 
-   if ((sock = open("/dev/bpf0", O_RDWR)) == -1)
+   if ((sock = open("/dev/bpf", O_RDWR)) == -1)
fatal("Can't open bpf device");
 
/* Set the BPF device to point at this interface. */
Index: usr.sbin/dhcrelay/bpf.c
===
RCS file: /cvs/src/usr.sbin/dhcrelay/bpf.c,v
retrieving revision 1.18
diff -u -p -r1.18 bpf.c
--- usr.sbin/dhcrelay/bpf.c 5 Apr 2017 14:40:56 -   1.18
+++ usr.sbin/dhcrelay/bpf.c 18 Apr 2017 18:31:56 -
@@ -72,7 +72,7 @@ if_register_bpf(struct interface_info *i
int sock;
 
/* Open the BPF device */
-   if ((sock = open("/dev/bpf0", O_RDWR)) == -1)
+   if ((sock = open("/dev/bpf", O_RDWR)) == -1)
fatal("Can't open bpf device");
 
/* Set the BPF device to point at this interface. */
Index: usr.sbin/dhcrelay6/bpf.c
===
RCS file: /cvs/src/usr.sbin/dhcrelay6/bpf.c,v
retrieving revision 1.1
diff -u -p -r1.1 bpf.c
--- usr.sbin/dhcrelay6/bpf.c17 Mar 2017 14:45:16 -  1.1
+++ usr.sbin/dhcrelay6/bpf.c18 Apr 2017 18:33:58 -
@@ -75,7 +75,7 @@ if_register_bpf(struct interface_info *i
int sock;
 
/* Open the BPF device */
-   if ((sock = open("/dev/bpf0", O_RDWR)) == -1)
+   if ((sock = open("/dev/bpf", O_RDWR)) == -1)
fatal("Can't open bpf device");
 
/* Set the BPF device to point at this interface. */
Index: usr.sbin/hostapd/hostapd.c
===
RCS file: /cvs/src/usr.sbin/hostapd/hostapd.c,v
retrieving revision 1.37
diff -u -p -r1.37 hostapd.c
--- usr.sbin/hostapd/hostapd.c  28 May 2016 07:00:18 -  1.37
+++ usr.sbin/hostapd/hostapd.c  18 Apr 2017 18:32:14 -
@@ -173,7 +173,7 @@ hostapd_bpf_open(u_int flags)
int fd = -1;
struct bpf_version bpv;
 
-   if ((fd = open("/dev/bpf0", flags)) == -1) {
+   if ((fd = open("/dev/bpf", flags)) == -1) {
hostapd_fatal("unable to open BPF device: %s\n",
strerror(errno));
}
Index: usr.sbin/mopd/common/pf.c

makefs filename handling

[Martin Natano]

Turns out there is no need to have a CD9660MAXPATH define. It is used to
construct the path for opening the file, so PATH_MAX makes more sense
here. While there I changed the code to do two less allocations per
file. Still seems to work fine.

Ok?

natano


Index: cd9660.c
===
RCS file: /cvs/src/usr.sbin/makefs/cd9660.c,v
retrieving revision 1.19
diff -u -p -r1.19 cd9660.c
--- cd9660.c17 Dec 2016 16:12:15 -  1.19
+++ cd9660.c30 Mar 2017 08:59:55 -
@@ -101,6 +101,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "makefs.h"
 #include "cd9660.h"
@@ -1453,16 +1454,22 @@ cd9660_generate_path_table(iso9660_disk 
return pathTableSize;
 }
 
-void
-cd9660_compute_full_filename(cd9660node *node, char *buf)
+char *
+cd9660_compute_full_filename(cd9660node *node)
 {
+   static char buf[PATH_MAX];
int len;
 
-   len = CD9660MAXPATH + 1;
-   len = snprintf(buf, len, "%s/%s/%s", node->node->root,
+   len = snprintf(buf, PATH_MAX, "%s/%s/%s", node->node->root,
node->node->path, node->node->name);
-   if (len > CD9660MAXPATH)
-   errx(1, "Pathname too long.");
+   if (len < 0) {
+   warn(NULL);
+   return NULL;
+   } else if (len >= PATH_MAX) {
+   warnc(ENAMETOOLONG, NULL);
+   return NULL;
+   }
+   return buf;
 }
 
 /*
Index: cd9660.h
===
RCS file: /cvs/src/usr.sbin/makefs/cd9660.h,v
retrieving revision 1.12
diff -u -p -r1.12 cd9660.h
--- cd9660.h17 Dec 2016 16:12:15 -  1.12
+++ cd9660.h29 Mar 2017 20:34:27 -
@@ -61,8 +61,6 @@
 #defineINODE_WARNX(__x)
 #endif /* DEBUG */
 
-#define CD9660MAXPATH 4096
-
 #define ISO_STRING_FILTER_NONE = 0x00
 #define ISO_STRING_FILTER_DCHARS = 0x01
 #define ISO_STRING_FILTER_ACHARS = 0x02
@@ -320,7 +318,7 @@ int cd9660_setup_boot_volume_descriptor(
 intcd9660_write_image(iso9660_disk *, const char *image);
 intcd9660_copy_file(iso9660_disk *, FILE *, off_t, const char *);
 
-void   cd9660_compute_full_filename(cd9660node *, char *);
+char   *cd9660_compute_full_filename(cd9660node *);
 intcd9660_compute_record_size(iso9660_disk *, cd9660node *);
 
 /* Debugging functions */
Index: cd9660/cd9660_write.c
===
RCS file: /cvs/src/usr.sbin/makefs/cd9660/cd9660_write.c,v
retrieving revision 1.7
diff -u -p -r1.7 cd9660_write.c
--- cd9660/cd9660_write.c   17 Dec 2016 16:22:04 -  1.7
+++ cd9660/cd9660_write.c   29 Mar 2017 20:38:09 -
@@ -234,19 +234,14 @@ cd9660_write_path_tables(iso9660_disk *d
 static int
 cd9660_write_file(iso9660_disk *diskStructure, FILE *fd, cd9660node *writenode)
 {
-   char *buf;
char *temp_file_name;
-   int ret;
off_t working_sector;
int cur_sector_offset;
iso_directory_record_cd9660 temp_record;
cd9660node *temp;
-   int rv = 0;
 
/* Todo : clean up variables */
 
-   temp_file_name = ecalloc(CD9660MAXPATH + 1, 1);
-   buf = emalloc(diskStructure->sectorSize);
if ((writenode->level != 0) &&
!(writenode->node->type & S_IFDIR)) {
fsinode *inode = writenode->node->inode;
@@ -258,12 +253,12 @@ cd9660_write_file(iso9660_disk *diskStru
INODE_WARNX(("%s: writing inode %d blocks at %" PRIu32,
__func__, (int)inode->st.st_ino, inode->ino));
inode->flags |= FI_WRITTEN;
-   cd9660_compute_full_filename(writenode,
-   temp_file_name);
-   ret = cd9660_copy_file(diskStructure, fd,
-   writenode->fileDataSector, temp_file_name);
-   if (ret == 0)
-   goto out;
+   temp_file_name = 
cd9660_compute_full_filename(writenode);
+   if (temp_file_name == NULL)
+   return 0;
+   if (cd9660_copy_file(diskStructure, fd,
+   writenode->fileDataSector, temp_file_name) == 0)
+   return 0;
}
} else {
/*
@@ -324,7 +319,7 @@ cd9660_write_file(iso9660_disk *diskStru
temp->su_tail_size, fd);
if (ferror(fd)) {
warnx("%s: write error", __func__);
-   goto out;
+   return 0;
}
cur_sector_offset += temp_record.length[0];
 
@@ -334,16 +329,11 @@ cd9660_write_file(iso9660_disk *diskStru
 * Recurse on children.
 */
TAILQ_FOREACH(temp, >cn_children, cn_next_child) {
-  

Re: mkdir(2): document EACCESS when no write permission

[Martin Natano]

On Thu, Mar 23, 2017 at 10:07:43AM -0600, Todd C. Miller wrote:
> From FreeBSD, POSIX has identical wording.
> 
> Alternately, we could add an extra EACCESS entry similar to what
> exists in open(2).  I'm happy with either approach.

OK natano@. I think using one entry is fine in this case as both
conditions are related to path resolution.

> 
>  - todd
> 
> Index: lib/libc/sys/mkdir.2
> ===
> RCS file: /cvs/src/lib/libc/sys/mkdir.2,v
> retrieving revision 1.17
> diff -u -p -u -r1.17 mkdir.2
> --- lib/libc/sys/mkdir.2  10 Sep 2015 17:55:21 -  1.17
> +++ lib/libc/sys/mkdir.2  23 Mar 2017 16:05:00 -
> @@ -101,7 +101,9 @@ bytes.
>  .It Bq Er ENOENT
>  A component of the path prefix does not exist.
>  .It Bq Er EACCES
> -Search permission is denied for a component of the path prefix.
> +Search permission is denied for a component of the path prefix,
> +or write permission is denied
> +on the parent directory of the directory to be created.
>  .It Bq Er ELOOP
>  Too many symbolic links were encountered in translating the pathname.
>  .It Bq Er EROFS
> 

Re: makefs: fix msdos create size

[Martin Natano]

On Wed, Mar 22, 2017 at 01:32:09PM +0100, Patrick Wildt wrote:
> 
> apparently the "create_size" option does currently not work.  This is
> because the strsuftoll() function uses "long long" compares which limits
> the positive maximum to LLONG_MAX.  Unfortunately the maximum is always
> set to ULLONG_MAX, which is treated as -1 and cannot be a reasonable
> maximum value.  This diff basically changes the maximum to LLONG_MAX.
> 
> Opinions? ok?

The two affected options (create_size and offset) are off_t, so your
diff makes sense. Go ahead.

> 
> Patrick
> 
> diff --git a/usr.sbin/makefs/msdos.c b/usr.sbin/makefs/msdos.c
> index a60af9adaa5..863349c09e4 100644
> --- a/usr.sbin/makefs/msdos.c
> +++ b/usr.sbin/makefs/msdos.c
> @@ -63,7 +63,7 @@ msdos_prep_opts(fsinfo_t *fsopts)
>   .minimum = _min,\
>   .maximum = sizeof(_type) == 1 ? 0xff :  \
>   (sizeof(_type) == 2 ? 0x :  \
> - (sizeof(_type) == 4 ? 0x : 0xLL)),  \
> + (sizeof(_type) == 4 ? 0x : 0x7fffLL)),  \
>  },
>  ALLOPTS
>  #undef AOPT
> 

Re: .depend permissions for libraries

[Martin Natano]

On Sun, Nov 06, 2016 at 12:56:31PM +0100, Theo Buehler wrote:
> The lib/*/obj/.depend files end up having permissions 600 since they are
> created as tempfiles and then moved to the obj directory.  I think that
> there is no deeper reason for such restrictive permissions and it gets
> in the way of having a dedicated build user.
> 
> If we copy the files and then remove the originals, the permissions of
> the .depend fiels are subject to the file creation rules, so they honor
> permissions of the obj directories and the umask.
> 
> ok?
> 
> Index: lib/csu/Makefile
> ===
> RCS file: /var/cvs/src/lib/csu/Makefile,v
> retrieving revision 1.24
> diff -u -p -r1.24 Makefile
> --- lib/csu/Makefile  15 Oct 2016 13:00:07 -  1.24
> +++ lib/csu/Makefile  6 Nov 2016 11:27:23 -
> @@ -85,9 +85,8 @@ afterdepend: .depend
>   @TMP=`mktemp .dependXX` || exit 1; \

This code comes from a time before sed -i existed. In this time and age
we can just use inline sed.

sed -i 's/^\([^\.]*\).o[ ]*:/\1.o g\1.o r\1.o:/' .depend

With this we avoid /tmp and the related permission issues altogether.
What do you think?


>   if sed -e 's/^\([^\.]*\).o[ ]*:/\1.o g\1.o r\1.o:/' \
> < .depend > $$TMP; then \
> - mv $$TMP .depend; \
> - else \
> - rm -f $$TMP; \
> - fi
> + cp $$TMP .depend; \
> + fi; \
> + rm -f $$TMP
>  
>  .include 
> Index: share/mk/bsd.lib.mk
> ===
> RCS file: /var/cvs/src/share/mk/bsd.lib.mk,v
> retrieving revision 1.78
> diff -u -p -r1.78 bsd.lib.mk
> --- share/mk/bsd.lib.mk   15 Oct 2016 13:00:07 -  1.78
> +++ share/mk/bsd.lib.mk   6 Nov 2016 11:18:12 -
> @@ -230,10 +230,9 @@ afterdepend: .depend
>   @TMP=`mktemp .dependXX` || exit 1; \

Same here.

sed -i 's/^\([^\.]*\).o[ ]*:/\1.o \1.po \1.so \1.do:/' .depend


>   if sed -e 's/^\([^\.]*\).o[ ]*:/\1.o \1.po \1.so \1.do:/' \
> < .depend > $$TMP; then \
> - mv $$TMP .depend; \
> - else \
> - rm -f $$TMP; \
> - fi
> + cp $$TMP .depend; \
> + fi ; \
> + rm -f $$TMP
>  .endif
>  
>  .if !target(install)
> 

Re: remove useless extern declaration

[Martin Natano]

On Wed, Oct 19, 2016 at 11:48:05PM +0200, Jan Stary wrote:
> extern char *optarg is already declared in unistd.h
> This is the only occurence in src/sbin and src/bin;
> others will follow in separate mails.
> 
>   Jan
> 

OK.


> 
> Index: bioctl.c
> ===
> RCS file: /cvs/src/sbin/bioctl/bioctl.c,v
> retrieving revision 1.139
> diff -u -p -r1.139 bioctl.c
> --- bioctl.c  21 Sep 2016 17:50:05 -  1.139
> +++ bioctl.c  19 Oct 2016 21:43:43 -
> @@ -59,7 +59,7 @@ struct timing {
>   int start;
>  };
>  
> -void usage(void);
> +static void __dead   usage(void);
>  const char   *str2locator(const char *, struct locator *);
>  const char   *str2patrol(const char *, struct timing *);
>  void bio_status(struct bio_status *);
> @@ -100,7 +100,6 @@ int
>  main(int argc, char *argv[])
>  {
>   struct bio_locate   bl;
> - extern char *optarg;
>   u_int64_t   func = 0;
>   char*devicename = NULL;
>   char*realname = NULL, *al_arg = NULL;
> @@ -273,7 +272,7 @@ main(int argc, char *argv[])
>   return (0);
>  }
>  
> -void
> +static void __dead
>  usage(void)
>  {
>   extern char *__progname;
> 

Re: config(8): use {err,warn}{,x} instead of fprintf(stderr, ...)

[Martin Natano]

On Sun, Oct 16, 2016 at 01:35:34PM +0200, Theo Buehler wrote:
> On Sun, Oct 16, 2016 at 11:47:32AM +0200, Theo Buehler wrote:
> > Many files already include  and there's a mix of hand-rolled
> > warning messages and there's incorrect usage warn("config: ..."). This
> > is a first sweep at unifying them.
> > 
> > In mkheaders.c, there is an err() function, rename it to emitwarn()
> > since there are emit* functions already and it is non-fatal.
> 
> Here's a slightly improved version: I failed to remove a few newlines in
> warning/error strings in main.c.

Looks good to me, but see some nits below.


> 
> Index: main.c
> ===
> RCS file: /cvs/src/usr.sbin/config/main.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 main.c
> --- main.c16 Oct 2016 09:36:46 -  1.55
> +++ main.c16 Oct 2016 11:25:20 -
> @@ -75,7 +75,7 @@ static void optiondelta(void);
>  
>  int  verbose;
>  
> -void
> +__dead void
>  usage(void)

The usage declaration in ukc.c needs updating too.

__dead void usage(void);


>  {
>   extern char *__progname;
> @@ -662,26 +656,14 @@ setupdirs(void)
>   builddir = defbuilddir;
>  
>   if (stat(builddir, ) != 0) {
> - if (mkdir(builddir, 0777)) {
> - (void)fprintf(stderr, "config: cannot create %s: %s\n",
> - builddir, strerror(errno));
> - exit(2);
> - }
> - } else if (!S_ISDIR(st.st_mode)) {
> - (void)fprintf(stderr, "config: %s is not a directory\n",
> - builddir);
> - exit(2);
> - }
> - if (chdir(builddir) != 0) {
> - (void)fprintf(stderr, "config: cannot change to %s\n",
> - builddir);
> - exit(2);
> - }
> - if (stat(srcdir, ) != 0 || !S_ISDIR(st.st_mode)) {
> - (void)fprintf(stderr, "config: %s is not a directory\n",
> - srcdir);
> - exit(2);
> - }
> + if (mkdir(builddir, 0777))
> + err(2, "cannot create %s:", builddir);

No ":" at the end of format. This will be added by err() automatically.


> + } else if (!S_ISDIR(st.st_mode))
> + errx(2, "%s is not a directory", builddir);
> + if (chdir(builddir) != 0)
> + errx(2, "cannot change to %s", builddir);
> + if (stat(srcdir, ) != 0 || !S_ISDIR(st.st_mode))
> + errx(2, "%s is not a directory", srcdir);
>  
>   if (bflag) {
>   if (pledge("stdio rpath wpath cpath flock", NULL) == -1)
> Index: mkheaders.c
> ===
> RCS file: /cvs/src/usr.sbin/config/mkheaders.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 mkheaders.c
> --- mkheaders.c   16 Jan 2015 06:40:16 -  1.21
> +++ mkheaders.c   16 Oct 2016 11:25:20 -
> @@ -102,22 +103,21 @@ emitcnt(struct nvlist *head)
>   nv = nv->nv_next;
>   }
>   if (ferror(fp))
> - return (err("read", fname, fp));
> + return (emitwarn("read", fname, fp));
>   (void)fclose(fp);
>   if (nv == NULL)
>   return (0);
>  writeit:
>   if ((fp = fopen(fname, "w")) == NULL) {
> - (void)fprintf(stderr, "config: cannot write %s: %s\n",
> - fname, strerror(errno));
> + warn("cannot write %s", fname);
>   return (1);

return(emitwarn("writ", fname, NULL)); ?


>   }
>   for (nv = head; nv != NULL; nv = nv->nv_next)
>   if (fprintf(fp, "#define\t%s\t%d\n",
>   cntname(nv->nv_name), nv->nv_int) < 0)
> - return (err("writ", fname, fp));
> + return (emitwarn("writ", fname, fp));
>   if (fclose(fp))
> - return (err("writ", fname, NULL));
> + return (emitwarn("writ", fname, NULL));
>   return (0);
>  }
>  
> @@ -175,23 +175,21 @@ writeit:
>* They're different, or the file doesn't exist.
>*/
>   if ((fp = fopen(fname, "w")) == NULL) {
> - (void)fprintf(stderr, "config: cannot write %s: %s\n",
> - fname, strerror(errno));
> + warn("cannot write %s", fname);
>   return (1);

return(emitwarn("writ", fname, NULL)); ?


>   }
>   if (fprintf(fp, "%s", new_contents) < 0)
> - return (err("writ", fname, fp));
> + return (emitwarn("writ", fname, fp));
>   if (fclose(fp))
> - return (err("writ", fname, fp));
> + return (emitwarn("writ", fname, fp));
>   return (0);
>  }
>  
>  static int
> -err(const char *what, char *fname, FILE *fp)
> +emitwarn(const char *what, char *fname, FILE *fp)
>  {
>  
> - (void)fprintf(stderr, "config: error %sing %s: %s\n",
> - what, fname, strerror(errno));
> + warn("error %sing %s", what, fname);
>   if (fp)
>   

remove config -g option

[Martin Natano]

The -g option is obsolete (and undocumented) at least since the initial
import in '95. Your finger memory should be updated by now.

Ok to remove?

natano


Index: main.c
===
RCS file: /cvs/src/usr.sbin/config/main.c,v
retrieving revision 1.53
diff -u -p -r1.53 main.c
--- main.c  16 Oct 2016 08:47:17 -  1.53
+++ main.c  16 Oct 2016 09:11:02 -
@@ -109,7 +109,7 @@ main(int argc, char *argv[])
err(1, "pledge");
 
pflag = eflag = uflag = fflag = 0;
-   while ((ch = getopt(argc, argv, "egpfb:s:o:u")) != -1) {
+   while ((ch = getopt(argc, argv, "epfb:s:o:u")) != -1) {
switch (ch) {
 
case 'o':
@@ -127,18 +127,6 @@ main(int argc, char *argv[])
if (!isatty(STDIN_FILENO))
verbose = 1;
break;
-
-   case 'g':
-   /*
-* In addition to DEBUG, you probably wanted to
-* set "options KGDB" and maybe others.  We could
-* do that for you, but you really should just
-* put them in the config file.
-*/
-   (void)fputs(
-   "-g is obsolete (use makeoptions DEBUG=\"-g\")\n",
-   stderr);
-   usage();
 
case 'p':
/*

config(8) getopt cleanup

[Martin Natano]

The declarations for optarg and optind are pulled in from . No
need to declare them again. Ok?

natano


Index: main.c
===
RCS file: /cvs/src/usr.sbin/config/main.c,v
retrieving revision 1.53
diff -u -p -r1.53 main.c
--- main.c  16 Oct 2016 08:47:17 -  1.53
+++ main.c  16 Oct 2016 08:58:07 -
@@ -58,9 +58,6 @@
 intfirstfile(const char *);
 intyyparse(void);
 
-extern char *optarg;
-extern int optind;
-
 static struct hashtab *mkopttab;
 static struct nvlist **nextopt;
 static struct nvlist **nextdefopt;

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

[Martin Natano]

On Thu, Oct 13, 2016 at 02:48:07AM +0200, Theo Buehler wrote:
> On Wed, Oct 12, 2016 at 10:30:26PM +0200, Theo Buehler wrote:
> > Several people noticed that a few files from libstdc++-v3 in /usr/obj
> > end up being owned by root, namely:
> > 
> > c++config.h gthr.h gthr-single.h gthr-posix.h gthr-tpf.h gthr-default.h
> > 
> > The problem is that they are regenerated by root when beforeinstall does
> > 'make includes' directly from /usr/src/includes.  A cheap fix is to call
> > make includes from the main Makefile, which was already fixed to
> > de-escalate the prereq step.
> > 
> > Fixing that directly in gnu/lib/libstdc++-v3/Makefile would also be
> > possible, but it seems to be quite a bit messier since one would have to
> > touch quite a few targets to do the proper de-escalation.
> > 
> > With that, /usr/obj contains no root-owned files after 'make build'.
> > 
> 
> Here's an improved version of the patch that doesn't interrupt
> 'make release' by asking for BUILDUSER's password because of trying
> to do 'sh -c ${BUILDUSER}' as BUILDUSER.
> 
> Index: Makefile
> ===
> RCS file: /var/cvs/src/Makefile,v
> retrieving revision 1.129
> diff -u -p -r1.129 Makefile
> --- Makefile  6 Oct 2016 18:56:17 -   1.129
> +++ Makefile  13 Oct 2016 00:19:13 -
> @@ -57,7 +57,11 @@ includes:
>  beforeinstall:
>   cd ${.CURDIR}/etc && exec ${MAKE} DESTDIR=${DESTDIR} distrib-dirs
>   cd ${.CURDIR}/etc && exec ${MAKE} DESTDIR=${DESTDIR} install-mtree
> - cd ${.CURDIR}/include && exec ${MAKE} includes
> + @if [[ `id -u` -ne 0 ]]; then \
> + cd ${.CURDIR}/include && exec ${MAKE} includes; \
> + else \
> + exec ${MAKE} includes; \
> + fi

I think that should be

exec ${MAKE} includes

with the conditional in the 'includes' target. Like this (untested):

includes:
cd ${.CURDIR}/include && \
if [[ `id -u` -eq 0 ]]; then \
su ${BUILDUSER} -c 'exec ${MAKE} prereq'; \
else \
exec ${MAKE} prereq; \
fi && \
exec ${MAKE} includes

That way 'prereq' is always run, also when started as unprivileged user,
and the 'includes' target can be started as BUILDUSER when DESTDIR is
pointing inside a noperm FS.

>  
>  afterinstall:
>  .ifndef NOMAN
> 

make(1): implement ${<D} and ${<F} as documented in the manual

[Martin Natano]

Diff below implements the ${ */
-   true,   /* $< */
true,   /* ${@F} */
true,   /* ${@D} */
false,  /* ${*F} */
@@ -177,6 +179,8 @@ static bool xtlist[] = {
false,  /* ${!D} */
true,   /* ${%F} */
true,   /* ${%D} */
+   true,   /* ${context.locals[idx])
 

Re: rework xenocara make includes step

[Martin Natano]

On Sun, Oct 09, 2016 at 02:32:12PM +0200, Matthieu Herrb wrote:
> Hi,
> 
> while looking at what is needed to switch the xenocara build system to
> the same kind of scheme as base (no more SUDO, start as root and drop
> privs to $BUILDUSER), I figured out that the global 'includes' step
> done at the begin of 'make build' is causing more trouble than it's
> solving...

Makes sense to me, OK. I believe the includes target in
distrib/notes/Makefile can also be removed when you commit this.
See one other comment below.

natano


> Index: share/mk/bsd.xorg.mk
> ===
> RCS file: /cvs/OpenBSD/xenocara/share/mk/bsd.xorg.mk,v
> retrieving revision 1.53
> diff -u -p -u -r1.53 bsd.xorg.mk
> --- share/mk/bsd.xorg.mk  28 Mar 2016 11:59:06 -  1.53
> +++ share/mk/bsd.xorg.mk  8 Oct 2016 19:23:31 -
> @@ -129,10 +129,6 @@ realinstall: install-headers-subdirs
>  .MAIN: all
>  .endif
>  
> -.if !target(includes)
> -includes: _SUBDIRUSE
> -.endif
> -
>  .if defined(SHARED_LIBS)
>  _lt_libs=
>  .for _n _v in ${SHARED_LIBS}
> @@ -211,6 +207,10 @@ build:
>   @exit 2
>  .else
>  build:
> +.if target(includes)
> + cd ${.CURDIR} && \
> + exec ${SUDO} ${MAKE} ${MAKE_FLAGS} ${_wrapper} includes
> +.endif

I think this is not be necessary. The only remaining 'includes' targets
are in freetype, libGLw and libepoxy. Those three take the header files
for the compile from the xenocara tree, not X11BASE. The headers will be
installed during 'make install' time anyway, so no need to do it again
before the compile.


>   cd ${.CURDIR} && exec ${MAKE} ${MAKE_FLAGS} ${_wrapper} cleandir
>   cd ${.CURDIR} && exec ${MAKE} ${MAKE_FLAGS} ${_wrapper} depend
>   cd ${.CURDIR} && exec ${MAKE} ${MAKE_FLAGS} ${_wrapper} all
> 
> -- 
> Matthieu Herrb

Re: libGLw/libexpoy: generate pkg-config files during build

[Martin Natano]

On Sat, Oct 08, 2016 at 04:16:47PM +0200, Matthieu Herrb wrote:
> 
> I would prefer to swith them to use the bsd.xorg.mk support for
> pkg-config files like that:

Yeah, that's much nicer. OK. See two comments below.


> 
> Index: lib/libGLw/Makefile
> ===
> RCS file: /cvs/OpenBSD/xenocara/lib/libGLw/Makefile,v
> retrieving revision 1.15
> diff -u -r1.15 Makefile
> --- lib/libGLw/Makefile   17 Aug 2012 15:16:44 -  1.15
> +++ lib/libGLw/Makefile   8 Oct 2016 14:15:07 -
> @@ -35,15 +35,12 @@
>  
>  beforeinstall: includes
>  
> -afterinstall: glw.pc
> - $(INSTALL) -c -m 644 -o root -g wheel glw.pc \
> - ${DESTDIR}${LIBDIR}/pkgconfig
> -
> -glw.pc: glw.pc.in
> - sed -e 's,@INSTALL_DIR@,$(X11BASE),' \
> - -e 's,@LIB_DIR@,lib,' \
> - -e 's,@VERSION@,$(MESA_MAJOR).$(MESA_MINOR).$(MESA_TINY),' \
> - < ${.CURDIR}/glw.pc.in > glw.pc
> +PKGCONFIG=   glw.pc
> +PACKAGE_VERSION=
> +EXTRA_PKGCONFIG_SUBST= \
> + -e 's,@INSTALL_DIR@,$(X11BASE),' \
> + -e 's,@LIB_DIR@,lib,' \
> + -e 's,@VERSION@,$(MESA_MAJOR).$(MESA_MINOR).$(MESA_TINY),' \

I think it would be cleaner to do

PACKAGE_VERSION= $(MESA_MAJOR).$(MESA_MINOR).$(MESA_TINY)

and use ${PACKAGE_VERSION} for the substitution here. We have to define
PACKAGE_VERSION anyway, so we might as well use it.


>  
>  NOPROFILE=
>  
> Index: lib/libepoxy/Makefile
> ===
> RCS file: /cvs/OpenBSD/xenocara/lib/libepoxy/Makefile,v
> retrieving revision 1.4
> diff -u -r1.4 Makefile
> --- lib/libepoxy/Makefile 1 Sep 2016 10:37:40 -   1.4
> +++ lib/libepoxy/Makefile 8 Oct 2016 14:15:07 -
> @@ -52,25 +52,15 @@
>  
>  NOPROFILE=
>  
> +PKGCONFIG=   epoxy.pc
> +PACKAGE_VERSION= $(EPOXY_MAJOR).$(EPOXY_MINOR)

Why no .${EPOXY_TINY} here?


> +EXTRA_PKGCONFIG_SUBST=   '-e s,@DLOPEN_LIBS@,,'
> +
>  obj: _xenocara_obj
>  
>  .include 
>  .include 
>  
> -epoxy.pc: ${EPOXY}/epoxy.pc.in Makefile
> - sed -e 's,@prefix@,$(X11BASE),' \
> - -e 's,@exec_prefix@,$(X11BASE),' \
> - -e 's,@libdir@,${LIBDIR},' \
> - -e 's,@includedir@,${INCSDIR},' \
> - -e 's,@DLOPEN_LIBS@,,' \
> - -e 's,@PACKAGE_VERSION@,$(EPOXY_MAJOR).$(EPOXY_MINOR),' \
> - < ${EPOXY}/epoxy.pc.in > epoxy.pc
> -
> -afterinstall: epoxy.pc
> - $(INSTALL) -c -m 644 -o root -g wheel epoxy.pc \
> - ${DESTDIR}${LIBDIR}/pkgconfig
> -
> -CLEANFILES+= epoxy.pc
> -
> +.PATH: ${EPOXY}
>  .PATH: ${EPOXY}/src
>  .PATH: ${.CURDIR}/generated/src
> 
> -- 
> Matthieu Herrb

libX11 ks_tables.h rebuild

[Martin Natano]

ks_tables.h is always considered out of date due to the forced rebuild
of the makekeys util. This means it's also rebuilt during install. First
as root during build, later by the BUILDUSER during release, which won't
be able to rewrite it, because it's now owned by root. With this result:

override rw-r--r--  root/wheel for ks_tables.h?


One step closer towards noperm release builds for xenocara.

Ok?

natano


Index: src/Makefile.am
===
RCS file: /cvs/xenocara/lib/libX11/src/Makefile.am,v
retrieving revision 1.13
diff -u -p -r1.13 Makefile.am
--- src/Makefile.am 6 Apr 2015 20:57:59 -   1.13
+++ src/Makefile.am 7 Oct 2016 21:15:08 -
@@ -416,7 +416,5 @@ ks_tables.h: $(KEYSYMDEFS) $(top_builddi
$(top_builddir)/src/util/makekeys $(KEYSYMDEFS) > ks_tables_h
mv ks_tables_h $@
 
-$(top_builddir)/src/util/makekeys$(EXEEXT): force
+$(top_builddir)/src/util/makekeys$(EXEEXT): $(top_builddir)/src/util/makekeys.c
cd util && $(MAKE)
-
-force:
Index: src/Makefile.in
===
RCS file: /cvs/xenocara/lib/libX11/src/Makefile.in,v
retrieving revision 1.22
diff -u -p -r1.22 Makefile.in
--- src/Makefile.in 6 Apr 2015 20:57:59 -   1.22
+++ src/Makefile.in 7 Oct 2016 21:15:08 -
@@ -1387,10 +1387,8 @@ ks_tables.h: $(KEYSYMDEFS) $(top_builddi
$(top_builddir)/src/util/makekeys $(KEYSYMDEFS) > ks_tables_h
mv ks_tables_h $@
 
-$(top_builddir)/src/util/makekeys$(EXEEXT): force
+$(top_builddir)/src/util/makekeys$(EXEEXT): $(top_builddir)/src/util/makekeys.c
cd util && $(MAKE)
-
-force:
 
 # Tell versions [3.59,3.63) of GNU make to not export all variables.
 # Otherwise a system limit (for SysV at least) may be exceeded.

libGLw/libexpoy: generate pkg-config files during build

[Martin Natano]

Generate the pkg-config files at build time, otherwise we might run into
permission issues with noperm release builds. While there properly clean
up glw.pc. Ok?

natano


Index: lib/libGLw/Makefile
===
RCS file: /cvs/xenocara/lib/libGLw/Makefile,v
retrieving revision 1.15
diff -u -p -r1.15 Makefile
--- lib/libGLw/Makefile 17 Aug 2012 15:16:44 -  1.15
+++ lib/libGLw/Makefile 7 Oct 2016 18:50:19 -
@@ -39,11 +39,14 @@ afterinstall: glw.pc
$(INSTALL) -c -m 644 -o root -g wheel glw.pc \
${DESTDIR}${LIBDIR}/pkgconfig
 
+all: glw.pc
 glw.pc: glw.pc.in
sed -e 's,@INSTALL_DIR@,$(X11BASE),' \
-e 's,@LIB_DIR@,lib,' \
-e 's,@VERSION@,$(MESA_MAJOR).$(MESA_MINOR).$(MESA_TINY),' \
< ${.CURDIR}/glw.pc.in > glw.pc
+
+CLEANFILES+= glw.pc
 
 NOPROFILE=
 
Index: lib/libepoxy/Makefile
===
RCS file: /cvs/xenocara/lib/libepoxy/Makefile,v
retrieving revision 1.4
diff -u -p -r1.4 Makefile
--- lib/libepoxy/Makefile   1 Sep 2016 10:37:40 -   1.4
+++ lib/libepoxy/Makefile   7 Oct 2016 18:50:19 -
@@ -57,6 +57,7 @@ obj: _xenocara_obj
 .include 
 .include 
 
+all: epoxy.pc
 epoxy.pc: ${EPOXY}/epoxy.pc.in Makefile
sed -e 's,@prefix@,$(X11BASE),' \
-e 's,@exec_prefix@,$(X11BASE),' \

Re: ps.1 tweak

[Martin Natano]

On Thu, Oct 06, 2016 at 09:27:21PM +0200, Michal Mazurek wrote:
> 
> Don't place a space after the minus sign. Change from this:
>   width in columns.  Otherwise, ps defaults to the terminal width
>   - 1, or 79 columns if none of stdout, stderr and stdin are a
> To this:
>   width in columns.  Otherwise, ps defaults to the terminal width
>   -1, or 79 columns if none of stdout, stderr and stdin are a

I think it is more readable the way it was before. As I understand it
the manpage doesn't refer to the constant '-1', but to 'terminal width
minus 1', so the additional space makes sense to me.

natano


> 
> Index: bin/ps/ps.1
> ===
> RCS file: /cvs/src/bin/ps/ps.1,v
> retrieving revision 1.109
> diff -u -p -r1.109 ps.1
> --- bin/ps/ps.1   23 Sep 2016 06:28:08 -  1.109
> +++ bin/ps/ps.1   6 Oct 2016 19:24:51 -
> @@ -543,7 +543,7 @@ If set to a positive integer,
>  output is formatted to the given width in columns.
>  Otherwise,
>  .Nm
> -defaults to the terminal width \(mi 1,
> +defaults to the terminal width \(mi1,
>  or 79 columns if none of
>  .Dv stdout ,
>  .Dv stderr
> 
> -- 
> Michal Mazurek
> 

Re: 'make build': simplify 'make includes' step

[Martin Natano]

On Thu, Oct 06, 2016 at 03:55:14PM +0200, Theo Buehler wrote:
> On Thu, Oct 06, 2016 at 03:09:49PM +0200, Martin Natano wrote:
> > 
> > Seems right to me. I wonder whether we should enforce root for includes
> > like we do for build. Either in the command list of the target, or even
> > something like this:
> 
> I'm somewhat torn.  Most, if not all, of the targets require root at
> some point, don't they?  Do we want to annotate all of them?  How much
> handholding do we really need to add to the Makefile?
> 
> Adding the warning to 'make build' and 'make release' made sense since
> these are targets that are used by many and they are going through a
> drastic transformation.

Fair point, I'd say go ahead with your original diff then. It's an
improvement to what we have now. We can add require_root markers if/when
we have decided where to put them.

natano

Re: 'make build': simplify 'make includes' step

[Martin Natano]

On Thu, Oct 06, 2016 at 02:02:41PM +0200, Theo Buehler wrote:
> Before calling 'make includes' during 'make build', the first two steps
> are repetitions of what a direct 'make includes' does anyway, so move
> the privdrop up there and simply call 'make includes' from 'make build'.
> 
> While there, use the exec idiom also for 'make cleandirs'.

Seems right to me. I wonder whether we should enforce root for includes
like we do for build. Either in the command list of the target, or even
something like this:


UID!= id -u
.if ${UID} == 0
includes:
# actual code

build:
# actual code
.else
build includes:
@echo make $@ must be called by root >&2
@false
.endif



or this:

(in bsd.own.mk)
require_root:
@if [[ `id -u` -ne 0 ]]; then \
echo $@ must be called by root >&2; \
false; \
fi
.PHONY: require_root

(in src/Makefile)
include: require_root
# actual code

build: require_root
# actual code


I thinke a require_root target in bsd.own.mk could be useful in other
parts of the tree too and it helps to keep the Makefile's clean from
those checks. What do you think?

natano


> 
> ok?
> 
> Index: Makefile
> ===
> RCS file: /var/cvs/src/Makefile,v
> retrieving revision 1.127
> diff -u -p -r1.127 Makefile
> --- Makefile  5 Oct 2016 18:00:41 -   1.127
> +++ Makefile  6 Oct 2016 11:31:46 -
> @@ -50,7 +50,9 @@ regression-tests:
>   @cd ${.CURDIR}/regress && ${MAKE} depend && exec ${MAKE} regress
>  
>  includes:
> - cd ${.CURDIR}/include && ${MAKE} prereq && exec ${MAKE} includes
> + cd ${.CURDIR}/include && \
> + su ${BUILDUSER} -c 'exec ${MAKE} prereq' && \
> + exec ${MAKE} includes
>  
>  beforeinstall:
>   cd ${.CURDIR}/etc && exec ${MAKE} DESTDIR=${DESTDIR} distrib-dirs
> @@ -77,10 +79,8 @@ build:
>   false; \
>   fi
>   cd ${.CURDIR}/share/mk && exec ${MAKE} install
> - cd ${.CURDIR}/include && \
> - su ${BUILDUSER} -c 'exec ${MAKE} prereq' && \
> - exec ${MAKE} includes
> - ${MAKE} cleandir
> + exec ${MAKE} includes
> + exec ${MAKE} cleandir
>   cd ${.CURDIR}/lib && \
>   su ${BUILDUSER} -c '${MAKE} depend && exec ${MAKE}' && \
>   NOMAN=1 exec ${MAKE} install
> 

Re: stricter sys_mount() flag handling

[Martin Natano]

> > And I want this check also for sys_unmount().
> > 
> 
> Good idea, sys_mount() is easy, because the only flag allowed there is
> MNT_FORCE.

s/sys_mount/sys_unmount/

stricter sys_mount() flag handling

[Martin Natano]

After committing the new MNT_NOPERM flag I got some complaints that my
code doesn't work by people that recompiled mount_ffs, but didn't reboot
to the new kernel. I don't blame them; in that situation sys_mount()
silently ignores the unknown flag. IMHO we should check the flags more
strictly. Ok?

natano


Index: sys/mount.h
===
RCS file: /cvs/src/sys/sys/mount.h,v
retrieving revision 1.127
diff -u -p -r1.127 mount.h
--- sys/mount.h 10 Sep 2016 16:53:30 -  1.127
+++ sys/mount.h 1 Oct 2016 15:36:11 -
@@ -414,6 +414,11 @@ struct mount {
 #define MNT_DOOMED 0x0800  /* device behind filesystem is gone */
 
 /*
+ * All mount flags.
+ */
+#defineMNT_FLAGMASK0x0e0f
+
+/*
  * Flags for various system call interfaces.
  *
  * waitfor flags to vfs_sync() and getfsstat()
Index: kern/vfs_syscalls.c
===
RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.265
diff -u -p -r1.265 vfs_syscalls.c
--- kern/vfs_syscalls.c 10 Sep 2016 16:53:30 -  1.265
+++ kern/vfs_syscalls.c 1 Oct 2016 15:36:11 -
@@ -117,6 +117,9 @@ sys_mount(struct proc *p, void *v, regis
if ((error = suser(p, 0)))
return (error);
 
+   if (flags & ~MNT_FLAGMASK)
+   return (EINVAL);
+
/*
 * Mount points must fit in MNAMELEN, not MAXPATHLEN.
 */

Re: cvs: owner of installed files

[Martin Natano]

> The diff doesn't apply because mkinstalldirs has an odd revision:
> 
> > Index: mkinstalldirs
> > ===
> > RCS file: /cvs/src/gnu/usr.bin/cvs/mkinstalldirs,v
> > retrieving revision 1.1.1.3
> > diff -u -p -r1.1.1.3 mkinstalldirs
> > --- mkinstalldirs   28 Sep 2001 22:45:35 -  1.1.1.3
> > +++ mkinstalldirs   17 Sep 2016 22:25:09 -
> > @@ -24,7 +24,7 @@ do
> >   if test ! -d "$pathcomp"; then
> >  echo "mkdir $pathcomp"
> >  
> > -mkdir "$pathcomp" || lasterr=$?
> > +install -d -o root -g wheel "$pathcomp" || lasterr=$?
> >  
> >  if test ! -d "$pathcomp"; then
> >   errstatus=$lasterr
> 
> I think you wanted to send the diff below.

Yes, thank you, I committed the version you included in your mail. Don't
know how I managed to botch the diff; I don't remember checking out an
old revision.

Re: rasops.c: avoid calculating offset several times

[Martin Natano]

On Mon, Sep 26, 2016 at 11:35:34AM +0200, Frederic Cambus wrote:
> Hi tech@,
> 
> Here is a diff to avoid calculating offset several times in rasops.c.
> This was done for a few functions already, but not all of them.
> 
> Comments? OK?

OK, that's more readable.


> 
> Index: sys/dev/rasops/rasops.c
> ===
> RCS file: /cvs/src/sys/dev/rasops/rasops.c,v
> retrieving revision 1.42
> diff -u -p -r1.42 rasops.c
> --- sys/dev/rasops/rasops.c   7 Sep 2015 18:00:58 -   1.42
> +++ sys/dev/rasops/rasops.c   21 Sep 2016 14:44:58 -
> @@ -1577,8 +1577,10 @@ rasops_vcons_erasecols(void *cookie, int
>   int i;
>  
>   for (i = 0; i < num; i++) {
> - scr->rs_bs[row * cols + col + i].uc = ' ';
> - scr->rs_bs[row * cols + col + i].attr = attr;
> + int off = row * cols + col + i;
> +
> + scr->rs_bs[off].uc = ' ';
> + scr->rs_bs[off].attr = attr;
>   }
>  
>   if (!scr->rs_visible)
> @@ -1626,8 +1628,10 @@ rasops_vcons_eraserows(void *cookie, int
>   int i;
>  
>   for (i = 0; i < num * cols; i++) {
> - scr->rs_bs[row * cols + i].uc = ' ';
> - scr->rs_bs[row * cols + i].attr = attr;
> + int off = row * cols + i;
> +
> + scr->rs_bs[off].uc = ' ';
> + scr->rs_bs[off].attr = attr;
>   }
>  
>   if (!scr->rs_visible)
> @@ -1695,8 +1699,10 @@ rasops_wronly_erasecols(void *cookie, in
>   int i;
>  
>   for (i = 0; i < num; i++) {
> - ri->ri_bs[row * cols + col + i].uc = ' ';
> - ri->ri_bs[row * cols + col + i].attr = attr;
> + int off = row * cols + col + i;
> +
> + ri->ri_bs[off].uc = ' ';
> + ri->ri_bs[off].attr = attr;
>   }
>  
>   return ri->ri_erasecols(ri, row, col, num, attr);
> @@ -1734,8 +1740,10 @@ rasops_wronly_eraserows(void *cookie, in
>   int i;
>  
>   for (i = 0; i < num * cols; i++) {
> - ri->ri_bs[row * cols + i].uc = ' ';
> - ri->ri_bs[row * cols + i].attr = attr;
> + int off = row * cols + i;
> +
> + ri->ri_bs[off].uc = ' ';
> + ri->ri_bs[off].attr = attr;
>   }
>  
>   return ri->ri_eraserows(ri, row, num, attr);
> 

Re: /usr/share/man owner fixes for noperm builds

[Martin Natano]

On Sun, Sep 25, 2016 at 01:14:17AM +0200, Theo Buehler wrote:
> usr/share/mandoc.db is currently installed as the build user:
> 
> -rw-r--r--   1 builder  wheel  396748 Sep 24 23:07 mandoc.db
> 
> bsd.own.mk defines MANOWN=root, MANGRP=bin, MANMODE=444 and uses these
> for installing the manpages themselves. Therefore I think we should use
> these variables rather than using BINOWN, BINGRP and hardcoded 444 for
> /usr/share/man/COPYRIGHT, and it makes more sense to me to have the
> mandoc.db owned by root:bin as well. The diff below does this.
> 
> However, the /etc/weekly script will change the ownership of mandoc.db
> to root:wheel (the current owner of the parent /usr/share/man), so I
> wonder if the entire man hierarchies should be switched to be owned by
> root:bin in the /etc/mtree/*BSD*.dist files. But that's a lot of churn
> and the benefit is unclear to me.
> 
> On the other hand, just doing 'chown root:wheel' for mandoc.db and be
> done with it would be a lot simpler...
> 
> Index: share/man/Makefile
> ===
> RCS file: /cvs/src/share/man/Makefile,v
> retrieving revision 1.10
> diff -u -p -r1.10 Makefile
> --- share/man/Makefile18 Apr 2014 10:00:48 -  1.10
> +++ share/man/Makefile24 Sep 2016 22:39:03 -
> @@ -4,10 +4,11 @@
>  SUBDIR=  man1 man3 man4 man5 man6 man7 man8 man9
>  
>  afterinstall:
> - ${INSTALL} -c -o ${BINOWN} -g ${BINGRP} -m 444 man0/COPYRIGHT \
> - ${DESTDIR}/usr/share/man/COPYRIGHT
> + ${INSTALL} ${INSTALL_COPY} -o ${MANOWN} -g ${MANGRP} -m ${MANMODE} \
> + man0/COPYRIGHT ${DESTDIR}/usr/share/man/COPYRIGHT
>  
>  makedb:
>   /usr/sbin/makewhatis -Qv ${DESTDIR}/usr/share/man
> + chown ${MANOWN}:${MANGRP} ${DESTDIR}/usr/share/man/mandoc.db

I think this should be root:wheel as long as /etc/weekly will reset it
to that. With root:wheel: OK.


>  
>  .include 
> 

libcrypto/Makefile CLEANFILES

[Martin Natano]

There are two lines in the Makefile setting CLEANFILES with the latter
one overriding the former. The result: libcrypto.pc is not removed on make
clean. Ok?

natano


Index: Makefile
===
RCS file: /cvs/src/lib/libcrypto/Makefile,v
retrieving revision 1.6
diff -u -p -r1.6 Makefile
--- Makefile14 Sep 2016 06:26:02 -  1.6
+++ Makefile23 Sep 2016 21:17:34 -
@@ -395,7 +395,7 @@ includes: obj_mac.h
 CFLAGS+= -I${.OBJDIR}
 
 GENERATED=obj_mac.h obj_dat.h
-CLEANFILES=${GENERATED} obj_mac.num.tmp
+CLEANFILES+=${GENERATED} obj_mac.num.tmp
 SSL_OBJECTS=${LCRYPTO_SRC}/objects
 
 obj_mac.h: ${SSL_OBJECTS}/objects.h ${SSL_OBJECTS}/obj_mac.num 
${SSL_OBJECTS}/objects.txt

LKMDIR in bsd.own.mk

[Martin Natano]

Loadable kernel modules are gone. Ok?

natano


Index: share/mk/bsd.own.mk
===
RCS file: /cvs/src/share/mk/bsd.own.mk,v
retrieving revision 1.178
diff -u -p -r1.178 bsd.own.mk
--- share/mk/bsd.own.mk 8 Sep 2016 18:59:49 -   1.178
+++ share/mk/bsd.own.mk 21 Sep 2016 17:49:40 -
@@ -73,11 +73,6 @@ DOCGRP?= bin
 DOCOWN?=   root
 DOCMODE?=  ${NONBINMODE}
 
-LKMDIR?=   /usr/lkm
-LKMGRP?=   ${BINGRP}
-LKMOWN?=   ${BINOWN}
-LKMMODE?=  ${NONBINMODE}
-
 LOCALEDIR?=/usr/share/locale
 LOCALEGRP?=wheel
 LOCALEOWN?=root

ownership fix for /usr/lib/locate/src.db

[Martin Natano]

Without this diff src.db is installed as the build user when makeing a
noperm release.  is required for BINOWN/BINGRP. Ok?

natano


Index: Makefile
===
RCS file: /cvs/src/distrib/sets/Makefile,v
retrieving revision 1.1
diff -u -p -r1.1 Makefile
--- Makefile9 Jul 2014 19:23:28 -   1.1
+++ Makefile20 Sep 2016 19:55:10 -
@@ -4,5 +4,8 @@ DB = /usr/lib/locate/src.db
 
 makedb:
/bin/sh ${.CURDIR}/makelocatedb ${OSrev} >${DESTDIR}${DB}
+   chown ${BINOWN}:${BINGRP} ${DESTDIR}${DB}
 
 .PHONY: makedb
+
+.include 

Re: share/: install ownership fixes

[Martin Natano]

Ping. Ok?


On Sat, Sep 10, 2016 at 10:16:20PM +0200, Martin Natano wrote:
> Another diff I typoed, also found by rpe@. Ok?
> 
> Index: share/misc/pcvtfonts/Makefile
> ===
> RCS file: /cvs/src/share/misc/pcvtfonts/Makefile,v
> retrieving revision 1.6
> diff -u -p -r1.6 Makefile
> --- share/misc/pcvtfonts/Makefile 13 May 2002 15:27:58 -  1.6
> +++ share/misc/pcvtfonts/Makefile 8 Sep 2016 20:54:08 -
> @@ -16,12 +16,9 @@ FONTDIR =  ${BINDIR}/misc/pcvtfonts
>  all: $(FONTS)
>  
>  install: ${FONTS}
> - @if [ ! -d ${DESTDIR}${FONTDIR} ]; then mkdir ${DESTDIR}${FONTDIR};fi
> - @for i in ${FONTS}; do \
> - echo "installing font $$i into ${DESTDIR}${FONTDIR}"; \
> - install -c -m ${LIBMODE} -o ${LIBOWN} -g ${LIBGRP} \
> - $$i ${DESTDIR}${FONTDIR}; \
> - done
> + ${INSTALL} -d -o root -g wheel ${DESTDIR}${FONTDIR}
> + ${INSTALL} ${INSTALL_COPY} -m ${LIBMODE} -o ${LIBOWN} -g ${LIBGRP} \
> + ${FONTS} ${DESTDIR}${FONTDIR}
>  
>  clean:
>   rm -f ${CLEANFILES}
> Index: share/snmp/Makefile
> ===
> RCS file: /cvs/src/share/snmp/Makefile,v
> retrieving revision 1.4
> diff -u -p -r1.4 Makefile
> --- share/snmp/Makefile   29 Jan 2016 03:06:00 -  1.4
> +++ share/snmp/Makefile   8 Sep 2016 21:02:02 -
> @@ -8,6 +8,7 @@ FILES+= OPENBSD-RELAYD-MIB.txt
>  all clean cleandir depend lint obj tags: _SUBDIRUSE
>  
>  realinstall:
> - ${INSTALL} -c -m 0444 ${FILES} ${DESTDIR}${BINDIR}/snmp/mibs
> + ${INSTALL} ${INSTALL_COPY} -o root -g wheel -m 0444 \
> + ${FILES} ${DESTDIR}${BINDIR}/snmp/mibs
>  
>  .include 
> Index: share/termtypes/Makefile
> ===
> RCS file: /cvs/src/share/termtypes/Makefile,v
> retrieving revision 1.24
> diff -u -p -r1.24 Makefile
> --- share/termtypes/Makefile  3 Dec 2015 11:30:46 -   1.24
> +++ share/termtypes/Makefile  10 Sep 2016 20:11:58 -
> @@ -14,12 +14,14 @@ termcap: termtypes.master
>   @[ -s ${.TARGET} ] || exit 1
>  
>  realinstall:
> + ${INSTALL} -d -o root -g wheel ${DESTDIR}${BINDIR}/terminfo
>   find terminfo -type f -exec \
>${INSTALL} -D ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 444 \
>{} ${DESTDIR}${BINDIR}/{} \;
>   ${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 444 termcap \
>${DESTDIR}${BINDIR}/misc/termcap
>   ln -fs ${BINDIR}/misc/termcap ${DESTDIR}/etc/termcap
> + chown -h root:wheel ${DESTDIR}/etc/termcap
>  
>  clean:
>   rm -f termcap

Re: libc: remove pointless off_t, size_t, void* and char* casts

[Martin Natano]

Reads fine to me. OK.

Re: little simpler ssh code

[Martin Natano]

Two more loops that can be converted to arc4random_buf(). Ok?

natano


Index: channels.c
===
RCS file: /cvs/src/usr.bin/ssh/channels.c,v
retrieving revision 1.352
diff -u -p -r1.352 channels.c
--- channels.c  12 Sep 2016 01:22:38 -  1.352
+++ channels.c  18 Sep 2016 19:04:30 -
@@ -4148,7 +4148,6 @@ x11_request_forwarding_with_spoofing(int
char *new_data;
int screen_number;
const char *cp;
-   u_int32_t rnd = 0;
 
if (x11_saved_display == NULL)
x11_saved_display = xstrdup(disp);
@@ -4175,15 +4174,12 @@ x11_request_forwarding_with_spoofing(int
 */
x11_saved_data = xmalloc(data_len);
x11_fake_data = xmalloc(data_len);
+   arc4random_buf(x11_fake_data, data_len);
for (i = 0; i < data_len; i++) {
if (sscanf(data + 2 * i, "%2x", ) != 1)
fatal("x11_request_forwarding: bad "
"authentication data: %.100s", data);
-   if (i % 4 == 0)
-   rnd = arc4random();
x11_saved_data[i] = value;
-   x11_fake_data[i] = rnd & 0xff;
-   rnd >>= 8;
}
x11_saved_data_len = data_len;
x11_fake_data_len = data_len;
Index: sshconnect1.c
===
RCS file: /cvs/src/usr.bin/ssh/sshconnect1.c,v
retrieving revision 1.78
diff -u -p -r1.78 sshconnect1.c
--- sshconnect1.c   15 Nov 2015 22:26:49 -  1.78
+++ sshconnect1.c   18 Sep 2016 19:04:30 -
@@ -504,7 +504,6 @@ ssh_kex(char *host, struct sockaddr *hos
u_char cookie[8];
u_int supported_ciphers;
u_int server_flags, client_flags;
-   u_int32_t rnd = 0;
 
debug("Waiting for server public key.");
 
@@ -563,12 +562,7 @@ ssh_kex(char *host, struct sockaddr *hos
 * random number, interpreted as a 32-byte key, with the least
 * significant 8 bits being the first byte of the key.
 */
-   for (i = 0; i < 32; i++) {
-   if (i % 4 == 0)
-   rnd = arc4random();
-   session_key[i] = rnd & 0xff;
-   rnd >>= 8;
-   }
+   arc4random_buf(session_key, SSH_SESSION_KEY_LENGTH);
 
/*
 * According to the protocol spec, the first byte of the session key

gnu/usr.bin/binutils*: install vs. ${INSTALL}

[Martin Natano]

I think we should be using ${INSTALL} here like in all the other
Makefile's. Ok?

natano


Index: binutils/Makefile.bsd-wrapper
===
RCS file: /cvs/src/gnu/usr.bin/binutils/Makefile.bsd-wrapper,v
retrieving revision 1.84
diff -u -p -r1.84 Makefile.bsd-wrapper
--- binutils/Makefile.bsd-wrapper   11 Sep 2016 07:42:02 -  1.84
+++ binutils/Makefile.bsd-wrapper   17 Sep 2016 22:35:36 -
@@ -81,8 +81,8 @@ install: maninstall
tooldir=${PREFIX} \
BSDSRCDIR=${BSDSRCDIR} \
INSTALL_MODULES='${INSTALL_MODULES}' \
-   INSTALL_PROGRAM='install -c -S ${INSTALL_STRIP} -o ${BINOWN} -g 
${BINGRP} -m ${BINMODE}' \
-   INSTALL_DATA='install -c -o ${DOCOWN} -g ${DOCGRP} -m 
${NONBINMODE}' \
+   INSTALL_PROGRAM='${INSTALL} -c -S ${INSTALL_STRIP} -o ${BINOWN} -g 
${BINGRP} -m ${BINMODE}' \
+   INSTALL_DATA='${INSTALL} -c -o ${DOCOWN} -g ${DOCGRP} -m 
${NONBINMODE}' \
INSTALL_INFO_HOST_MODULES='${INSTALL_INFO_HOST_MODULES}' \
  install install-info
 
Index: binutils-2.17/Makefile.bsd-wrapper
===
RCS file: /cvs/src/gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper,v
retrieving revision 1.9
diff -u -p -r1.9 Makefile.bsd-wrapper
--- binutils-2.17/Makefile.bsd-wrapper  11 Sep 2016 07:42:02 -  1.9
+++ binutils-2.17/Makefile.bsd-wrapper  17 Sep 2016 22:35:38 -
@@ -102,8 +102,8 @@ install: maninstall
tooldir=${PREFIX} \
BSDSRCDIR=${BSDSRCDIR} \
INSTALL_MODULES='${INSTALL_MODULES}' \
-   INSTALL_PROGRAM='install -c -S ${INSTALL_STRIP} -o ${BINOWN} -g 
${BINGRP} -m ${BINMODE}' \
-   INSTALL_DATA='install -c -o ${DOCOWN} -g ${DOCGRP} -m 
${NONBINMODE}' \
+   INSTALL_PROGRAM='${INSTALL} -c -S ${INSTALL_STRIP} -o ${BINOWN} -g 
${BINGRP} -m ${BINMODE}' \
+   INSTALL_DATA='${INSTALL} -c -o ${DOCOWN} -g ${DOCGRP} -m 
${NONBINMODE}' \
INSTALL_INFO_HOST_MODULES='${INSTALL_INFO_HOST_MODULES}' \
  install install-info
 

cvs: owner of installed files

[Martin Natano]

Next round of wrestling with install permissions. This diff adjusts the
file owner/group for installed files of cvs(1). Ok?

natano


Index: Makefile.bsd-wrapper
===
RCS file: /cvs/src/gnu/usr.bin/cvs/Makefile.bsd-wrapper,v
retrieving revision 1.52
diff -u -p -r1.52 Makefile.bsd-wrapper
--- Makefile.bsd-wrapper21 Oct 2014 00:12:46 -  1.52
+++ Makefile.bsd-wrapper17 Sep 2016 22:25:09 -
@@ -28,21 +28,23 @@ CF=
 config: .FORCE
-rm -f config.cache
PATH="/bin:/usr/bin:/sbin:/usr/sbin" \
-   INSTALL_PROGRAM="${INSTALL} ${INSTALL_COPY} ${INSTALL_STRIP}" \
-   INSTALL_SCRIPT="${INSTALL} ${INSTALL_COPY}" \
-   ACLOCAL=true AUTOCONF=true AUTOMAKE=true AUTOHEADER=true \
-   MAKEINFO='makeinfo --no-split' \
-   sh ${.CURDIR}/configure --prefix=/usr --mandir=/usr/share/man \
-   --datadir=/usr/libdata ${CF}
+   INSTALL_PROGRAM="${INSTALL} ${INSTALL_COPY} ${INSTALL_STRIP} -o 
${BINOWN} -g ${BINGRP} -m ${BINMODE}" \
+   INSTALL_SCRIPT="${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 
${BINMODE}" \
+   INSTALL_DATA="${INSTALL} ${INSTALL_COPY} -o ${DOCOWN} -g ${DOCGRP} -m 
${DOCMODE}" \
+   ACLOCAL=true AUTOCONF=true AUTOMAKE=true AUTOHEADER=true \
+   MAKEINFO='makeinfo --no-split' \
+   sh ${.CURDIR}/configure --prefix=/usr --mandir=/usr/share/man \
+   --datadir=/usr/libdata ${CF}
 
 config.status:
PATH="/bin:/usr/bin:/sbin:/usr/sbin" \
-   INSTALL_PROGRAM="${INSTALL} ${INSTALL_COPY} ${INSTALL_STRIP}" \
-   INSTALL_SCRIPT="${INSTALL} ${INSTALL_COPY}" \
-   ACLOCAL=true AUTOCONF=true AUTOMAKE=true AUTOHEADER=true \
-   MAKEINFO='makeinfo --no-split' \
-   sh ${.CURDIR}/configure --prefix=/usr --mandir=/usr/share/man \
-   --datadir=/usr/libdata ${CF}
+   INSTALL_PROGRAM="${INSTALL} ${INSTALL_COPY} ${INSTALL_STRIP} -o 
${BINOWN} -g ${BINGRP} -m ${BINMODE}" \
+   INSTALL_SCRIPT="${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 
${BINMODE}" \
+   INSTALL_DATA="${INSTALL} ${INSTALL_COPY} -o ${DOCOWN} -g ${DOCGRP} -m 
${DOCMODE}" \
+   ACLOCAL=true AUTOCONF=true AUTOMAKE=true AUTOHEADER=true \
+   MAKEINFO='makeinfo --no-split' \
+   sh ${.CURDIR}/configure --prefix=/usr --mandir=/usr/share/man \
+   --datadir=/usr/libdata ${CF}
 
 .ifdef NOMAN
 maninstall:
Index: mkinstalldirs
===
RCS file: /cvs/src/gnu/usr.bin/cvs/mkinstalldirs,v
retrieving revision 1.1.1.3
diff -u -p -r1.1.1.3 mkinstalldirs
--- mkinstalldirs   28 Sep 2001 22:45:35 -  1.1.1.3
+++ mkinstalldirs   17 Sep 2016 22:25:09 -
@@ -24,7 +24,7 @@ do
  if test ! -d "$pathcomp"; then
 echo "mkdir $pathcomp"
 
-mkdir "$pathcomp" || lasterr=$?
+install -d -o root -g wheel "$pathcomp" || lasterr=$?
 
 if test ! -d "$pathcomp"; then
  errstatus=$lasterr
Index: contrib/Makefile.in
===
RCS file: /cvs/src/gnu/usr.bin/cvs/contrib/Makefile.in,v
retrieving revision 1.13
diff -u -p -r1.13 Makefile.in
--- contrib/Makefile.in 10 Jun 2003 21:06:17 -  1.13
+++ contrib/Makefile.in 17 Sep 2016 22:25:09 -
@@ -363,7 +363,8 @@ install-data-local:
echo "test ! -e $(DESTDIR)$(bindir)/`echo $$p|sed '$(transform)'`"; 
\
echo "  && cd $(DESTDIR)$(bindir) && $(LN_S) 
$(contribscriptdir)/`echo $$p|sed '$(transform)'` ."; \
(test ! -e $(DESTDIR)$(bindir)/`echo $$p|sed '$(transform)'` \
-   && cd $(DESTDIR)$(bindir) && $(LN_S) $(contribscriptdir)/`echo 
$$p|sed '$(transform)'` .) \
+   && cd $(DESTDIR)$(bindir) && $(LN_S) $(contribscriptdir)/`echo 
$$p|sed '$(transform)'` . \
+   && chown root:bin `echo $$p|sed '$(transform)'`) \
  || (echo "Link creation failed" && if test -f $$p; then \
   echo " $(INSTALL_SCRIPT) $$p $(DESTDIR)$(bindir)/`echo 
$$p|sed '$(transform)'`"; \
   $(INSTALL_SCRIPT) $$p $(DESTDIR)$(bindir)/`echo $$p|sed 
'$(transform)'`; \

Re: little simpler ssh code

[Martin Natano]

On Fri, Sep 16, 2016 at 09:19:40PM -0400, Ted Unangst wrote:
> no change, but makes the code a little shorter.

Reads fine to me. arc4random() is already used in other places in ssh,
so this shouldn't be an issue for portable.


> 
> 
> Index: clientloop.c
> ===
> RCS file: /cvs/src/usr.bin/ssh/clientloop.c,v
> retrieving revision 1.287
> diff -u -p -r1.287 clientloop.c
> --- clientloop.c  12 Sep 2016 01:22:38 -  1.287
> +++ clientloop.c  17 Sep 2016 01:16:46 -
> @@ -303,7 +303,7 @@ client_x11_get_proto(const char *display
>   char xauthfile[PATH_MAX], xauthdir[PATH_MAX];
>   static char proto[512], data[512];
>   FILE *f;
> - int got_data = 0, generated = 0, do_unlink = 0, i, r;
> + int got_data = 0, generated = 0, do_unlink = 0, r;
>   struct stat st;
>   u_int now, x11_timeout_real;
>  
> @@ -430,17 +430,16 @@ client_x11_get_proto(const char *display
>* for the local connection.
>*/
>   if (!got_data) {
> - u_int32_t rnd = 0;
> + u_int8_t rnd[16];
> + u_int i;
>  
>   logit("Warning: No xauth data; "
>   "using fake authentication data for X11 forwarding.");
>   strlcpy(proto, SSH_X11_PROTO, sizeof proto);
> - for (i = 0; i < 16; i++) {
> - if (i % 4 == 0)
> - rnd = arc4random();
> + arc4random_buf(rnd, sizeof(rnd));
> + for (i = 0; i < sizeof(rnd); i++) {
>   snprintf(data + 2 * i, sizeof data - 2 * i, "%02x",
> - rnd & 0xff);
> - rnd >>= 8;
> + rnd[i]);
>   }
>   }
>  
> 

Re: binutils-2.17 ownership fixes

[Martin Natano]

On Thu, Sep 15, 2016 at 09:07:49PM -0700, Philip Guenther wrote:
> On Thu, Sep 15, 2016 at 1:58 PM, Martin Natano <nat...@natano.net> wrote:
> > This should do it. The 'fix' is ugly, but I couldn't find a cleaner way
> > to pass the right STRIP value to libtool. Any better ideas? Ok?
> ...
> > --- gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper  11 Sep 2016 
> > 07:42:02 -  1.9
> > +++ gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper  15 Sep 2016 
> > 20:56:45 -
> > @@ -80,6 +80,8 @@ do-config: .USE
> > mv -f Makefile.tmp Makefile
> > cd ${.OBJDIR} && \
> > ${MAKE} ${CONFIGURE_MODULES}
> > +   sed -i 's,^STRIP=strip$$,STRIP=/usr/bin/strip,' \
> > +   ${.OBJDIR}/binutils/libtool
> 
> Instead of hacking the generated libtool post-facto, maybe just
> hardcode a usable value into what generates the script?
> 
> 
> --- ltconfig24 Apr 2011 20:14:40 -  1.1.1.1
> +++ ltconfig16 Sep 2016 03:44:50 -
> @@ -2331,7 +2331,7 @@ LN_S=$LN_S
>  NM=$NM
> 
>  # A symbol stripping program
> -STRIP=$STRIP
> +STRIP=/usr/bin/strip
> 
>  # Used to examine libraries when file_magic_cmd begins "file"
>  MAGIC_CMD=$MAGIC_CMD

That's much nicer. OK, please commit.

Re: binutils-2.17 ownership fixes

[Martin Natano]

> install: strip: No such file or directory
> 
> Is there a better option than removing it again?
> 
> 
> $ cd /usr/src/gnu/usr.bin/binutils-2.17
> $ doas make -f Makefile.bsd-wapper install
> [...]
> Making install in po
> test -z "/usr/bin" || /bin/sh 
> /usr/src/gnu/usr.bin/binutils-2.17/binutils/../mkinstalldirs "/usr/bin"
>   /bin/sh ./libtool --mode=install install -c -S -s -o root -g bin -m 555 
> 'objdump' '/usr/bin/objdump'
> install -c -S -o root -g bin -m 555 -s objdump /usr/bin/objdump
> install: strip: No such file or directory
>   /bin/sh ./libtool --mode=install install -c -S -s -o root -g bin -m 555 
> 'ar' '/usr/bin/ar'
> install -c -S -o root -g bin -m 555 -s ar /usr/bin/ar
> install: strip: No such file or directory
>   /bin/sh ./libtool --mode=install install -c -S -s -o root -g bin -m 555 
> 'strings' '/usr/bin/strings'
> install -c -S -o root -g bin -m 555 -s strings /usr/bin/strings
> install: strip: No such file or directory
>   /bin/sh ./libtool --mode=install install -c -S -s -o root -g bin -m 555 
> 'ranlib' '/usr/bin/ranlib'
> install -c -S -o root -g bin -m 555 -s ranlib /usr/bin/ranlib
> install: strip: No such file or directory
>   /bin/sh ./libtool --mode=install install -c -S -s -o root -g bin -m 555 
> 'objcopy' '/usr/bin/objcopy'
> install -c -S -o root -g bin -m 555 -s objcopy /usr/bin/objcopy
> install: strip: No such file or directory
>   /bin/sh ./libtool --mode=install install -c -S -s -o root -g bin -m 555 
> 'addr2line' '/usr/bin/addr2line'
> install -c -S -o root -g bin -m 555 -s addr2line /usr/bin/addr2line
> install: strip: No such file or directory
>   /bin/sh ./libtool --mode=install install -c -S -s -o root -g bin -m 555 
> 'readelf' '/usr/bin/readelf'
> install -c -S -o root -g bin -m 555 -s readelf /usr/bin/readelf
> install: strip: No such file or directory
>  /bin/sh ./libtool  --mode=install install -c -S -s -o root -g bin -m 555 
> strip-new /usr/bin/strip
> install -c -S -o root -g bin -m 555 -s strip-new /usr/bin/strip
> install: strip: No such file or directory
> [...]
> 

This should do it. The 'fix' is ugly, but I couldn't find a cleaner way
to pass the right STRIP value to libtool. Any better ideas? Ok?

Index: gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper
===
RCS file: /cvs/src/gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper,v
retrieving revision 1.9
diff -u -p -r1.9 Makefile.bsd-wrapper
--- gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper  11 Sep 2016 07:42:02 
-  1.9
+++ gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper  15 Sep 2016 20:56:45 
-
@@ -80,6 +80,8 @@ do-config: .USE
mv -f Makefile.tmp Makefile
cd ${.OBJDIR} && \
${MAKE} ${CONFIGURE_MODULES}
+   sed -i 's,^STRIP=strip$$,STRIP=/usr/bin/strip,' \
+   ${.OBJDIR}/binutils/libtool
 
 gas/doc/as.1: config.status
cd ${.OBJDIR}/gas/doc && ${MAKE} as.1

generate pkg-config files at build time

[Martin Natano]

Currently pkg-config files are generated at install time, while they
should be generated at build time like everything else. One reason why
generating files during install is bad is that the two steps might be
run by two differnt users, resulting in permission problems.

While there I removed the dependency from the install target on the
pkg-config files as this is also not done for libraries and programs.
If you didn't build before install you are fucked anyway.

Ok?

natano


Index: lib/libcrypto/Makefile
===
RCS file: /cvs/src/lib/libcrypto/Makefile,v
retrieving revision 1.5
diff -u -p -r1.5 Makefile
--- lib/libcrypto/Makefile  11 Sep 2016 14:31:02 -  1.5
+++ lib/libcrypto/Makefile  12 Sep 2016 18:19:53 -
@@ -431,10 +431,11 @@ distribution:
${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 444 \
   ${.CURDIR}/x509v3.cnf ${DESTDIR}/etc/ssl/x509v3.cnf
 
+all: ${PC_FILES}
 ${PC_FILES}: opensslv.h
/bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
 
-beforeinstall: ${PC_FILES}
+beforeinstall:
${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
-m ${SHAREMODE} ${.OBJDIR}/${PC_FILES} ${DESTDIR}/usr/lib/pkgconfig/
 
Index: lib/libexpat/Makefile
===
RCS file: /cvs/src/lib/libexpat/Makefile,v
retrieving revision 1.10
diff -u -p -r1.10 Makefile
--- lib/libexpat/Makefile   4 Sep 2016 09:54:25 -   1.10
+++ lib/libexpat/Makefile   12 Sep 2016 18:20:08 -
@@ -17,10 +17,11 @@ includes:
  ${INSTALL} ${INSTALL_COPY} -m 444 -o $(BINOWN) -g $(BINGRP) \
  ${.CURDIR}/lib/expat_external.h 
${DESTDIR}/usr/include/expat_external.h
 
+all: ${PC_FILES}
 ${PC_FILES}: lib/expat.h
/bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
 
-beforeinstall: ${PC_FILES}
+beforeinstall:
${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
-m ${SHAREMODE} ${.OBJDIR}/${PC_FILES} ${DESTDIR}/usr/lib/pkgconfig/
 
Index: lib/libfuse/Makefile
===
RCS file: /cvs/src/lib/libfuse/Makefile,v
retrieving revision 1.9
diff -u -p -r1.9 Makefile
--- lib/libfuse/Makefile4 Sep 2016 09:54:25 -   1.9
+++ lib/libfuse/Makefile12 Sep 2016 18:20:16 -
@@ -29,10 +29,11 @@ includes:
eval "$$j"; \
done
 
+all: ${PC_FILES}
 ${PC_FILES}: fuse_private.h
/bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
 
-beforeinstall: ${PC_FILES}
+beforeinstall:
${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
-m ${SHAREMODE} ${.OBJDIR}/${PC_FILES} ${DESTDIR}/usr/lib/pkgconfig/
 
Index: lib/libssl/Makefile
===
RCS file: /cvs/src/lib/libssl/Makefile,v
retrieving revision 1.21
diff -u -p -r1.21 Makefile
--- lib/libssl/Makefile 4 Sep 2016 09:54:25 -   1.21
+++ lib/libssl/Makefile 12 Sep 2016 18:20:34 -
@@ -48,10 +48,11 @@ includes:
 
 .include 
 
+all: ${PC_FILES}
 ${PC_FILES}: ${.CURDIR}/../libcrypto/opensslv.h
/bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
 
-beforeinstall: ${PC_FILES}
+beforeinstall:
nm -o lib${LIB}.a | egrep -w 'printf|fprintf' && \
(echo please fix stdio usage in this library; false) || true
 .for p in ${PC_FILES}
Index: lib/libz/Makefile
===
RCS file: /cvs/src/lib/libz/Makefile,v
retrieving revision 1.19
diff -u -p -r1.19 Makefile
--- lib/libz/Makefile   4 Sep 2016 09:54:25 -   1.19
+++ lib/libz/Makefile   12 Sep 2016 18:20:44 -
@@ -19,10 +19,11 @@ includes:
eval "$$j"; \
done
 
+all: ${PC_FILES}
 ${PC_FILES}: zlib.h
/bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
 
-beforeinstall: ${PC_FILES}
+beforeinstall:
${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
-m ${SHAREMODE} ${.OBJDIR}/${PC_FILES} ${DESTDIR}/usr/lib/pkgconfig/
 

texinfo ownership fix

[Martin Natano]

Similar to guenther's fix for gnu/binutils*. Ok?

natano


Index: gnu/usr.bin/texinfo/Makefile.bsd-wrapper
===
RCS file: /cvs/src/gnu/usr.bin/texinfo/Makefile.bsd-wrapper,v
retrieving revision 1.42
diff -u -p -r1.42 Makefile.bsd-wrapper
--- gnu/usr.bin/texinfo/Makefile.bsd-wrapper11 Jul 2014 23:23:28 -  
1.42
+++ gnu/usr.bin/texinfo/Makefile.bsd-wrapper11 Sep 2016 12:12:03 -
@@ -34,13 +34,17 @@ config: .FORCE
 .endif
PATH="/bin:/usr/bin:/sbin:/usr/sbin" \
${XCFLAGS} \
-   INSTALL_PROGRAM="${INSTALL} ${INSTALL_COPY} ${INSTALL_STRIP}" \
+   INSTALL_PROGRAM="${INSTALL} ${INSTALL_COPY} ${INSTALL_STRIP} -o 
${BINOWN} -g ${BINGRP} -m ${BINMODE}" \
+   INSTALL_SCRIPT="${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 
${BINMODE}" \
+   INSTALL_DATA="${INSTALL} ${INSTALL_COPY} -o ${DOCOWN} -g ${DOCGRP}  -m 
${NONBINMODE}" \
/bin/sh ${.CURDIR}/configure --infodir=/usr/share/info 
--prefix=/usr --disable-nls ${CF}
 
 config.status:
PATH="/bin:/usr/bin:/sbin:/usr/sbin" \
${XCFLAGS} \
-   INSTALL_PROGRAM="${INSTALL} ${INSTALL_COPY} ${INSTALL_STRIP}" \
+   INSTALL_PROGRAM="${INSTALL} ${INSTALL_COPY} ${INSTALL_STRIP} -o 
${BINOWN} -g ${BINGRP}  -m ${BINMODE}" \
+   INSTALL_SCRIPT="${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 
${BINMODE}" \
+   INSTALL_DATA="${INSTALL} ${INSTALL_COPY} -o ${DOCOWN} -g ${DOCGRP} -m 
${NONBINMODE}" \
/bin/sh ${.CURDIR}/configure --infodir=/usr/share/info 
--prefix=/usr --disable-nls ${CF}
 
 BEFOREMAN=config.status

/etc/ssl/* owner

[Martin Natano]

Following diff installs the files in /etc/ssl with the correct owner.
Ok?

natano


Index: lib/libcrypto/Makefile
===
RCS file: /cvs/src/lib/libcrypto/Makefile,v
retrieving revision 1.4
diff -u -p -r1.4 Makefile
--- lib/libcrypto/Makefile  4 Sep 2016 17:59:26 -   1.4
+++ lib/libcrypto/Makefile  11 Sep 2016 11:45:25 -
@@ -424,11 +424,11 @@ all beforedepend: ${GENERATED}
 
 
 distribution:
-   ${INSTALL} ${INSTALL_COPY} -g ${BINGRP} -m 444 \
+   ${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 444 \
   ${.CURDIR}/openssl.cnf ${DESTDIR}/etc/ssl/openssl.cnf && \
-   ${INSTALL} ${INSTALL_COPY} -g ${BINGRP} -m 444 \
+   ${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 444 \
   ${.CURDIR}/cert.pem ${DESTDIR}/etc/ssl/cert.pem && \
-   ${INSTALL} ${INSTALL_COPY} -g ${BINGRP} -m 444 \
+   ${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 444 \
   ${.CURDIR}/x509v3.cnf ${DESTDIR}/etc/ssl/x509v3.cnf
 
 ${PC_FILES}: opensslv.h
Index: usr.sbin/ikectl/Makefile
===
RCS file: /cvs/src/usr.sbin/ikectl/Makefile,v
retrieving revision 1.6
diff -u -p -r1.6 Makefile
--- usr.sbin/ikectl/Makefile1 Mar 2016 13:54:39 -   1.6
+++ usr.sbin/ikectl/Makefile11 Sep 2016 11:45:26 -
@@ -16,7 +16,7 @@ CFLAGS+=  -Wshadow -Wpointer-arith -Wcast
 CFLAGS+=   -Wsign-compare
 
 distribution:
-   ${INSTALL} -C -g wheel -m 0644 ${.CURDIR}/ikeca.cnf \
+   ${INSTALL} -C -o root -g wheel -m 0644 ${.CURDIR}/ikeca.cnf \
${DESTDIR}/etc/ssl/ikeca.cnf
 
 .include 

sqlite3 headers owner

[Martin Natano]

The sqlite3 headers are installed with the build user as owner, but
should be owned by root. Ok?

natano


Index: Makefile
===
RCS file: /cvs/src/lib/libsqlite3/Makefile,v
retrieving revision 1.15
diff -u -p -r1.15 Makefile
--- Makefile12 Sep 2015 02:08:34 -  1.15
+++ Makefile11 Sep 2016 11:22:24 -
@@ -104,7 +104,8 @@ beforeinstall:
 includes:
@for i in ${FILES}; do \
cmp -s ${.CURDIR}/src/$$i ${DESTDIR}/usr/include/$$i || \
-   ${INSTALL} ${INSTALL_COPY} -m 444 ${.CURDIR}/src/$$i 
${DESTDIR}/usr/include/$$i; \
+   ${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 444 \
+   ${.CURDIR}/src/$$i ${DESTDIR}/usr/include/$$i; \
done
 
 .PHONY: header

/usr/bin/skeyprune owner

[Martin Natano]

Another ${INSTALL} invokation that doesn't mention the owner. Ok?

natano


Index: Makefile
===
RCS file: /cvs/src/usr.bin/skey/Makefile,v
retrieving revision 1.15
diff -u -p -r1.15 Makefile
--- Makefile30 Mar 2016 06:38:46 -  1.15
+++ Makefile11 Sep 2016 07:52:23 -
@@ -9,7 +9,7 @@ DPADD=  ${LIBSKEY}
 LDADD= -lskey
 
 beforeinstall:
-   ${INSTALL} ${INSTALL_COPY} -m 755 ${.CURDIR}/skeyprune.pl \
-   ${DESTDIR}${BINDIR}/skeyprune
+   ${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 755 \
+   ${.CURDIR}/skeyprune.pl ${DESTDIR}${BINDIR}/skeyprune
 
 .include 

Re: binutils-2.17 ownership fixes

[Martin Natano]

> Index: gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper
> ===
> RCS file: 
> /data/src/openbsd/src/gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper,v
> retrieving revision 1.8
> diff -u -p -r1.8 Makefile.bsd-wrapper
> --- gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper5 Jul 2013 21:29:51 
> -   1.8
> +++ gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper11 Sep 2016 01:54:10 
> -
> @@ -102,7 +102,8 @@ install: maninstall
>   tooldir=${PREFIX} \
>   BSDSRCDIR=${BSDSRCDIR} \
>   INSTALL_MODULES='${INSTALL_MODULES}' \
> - INSTALL_PROGRAM='install -c -S' \
> + INSTALL_PROGRAM='install -c -S ${INSTALL_STRIP} -o ${BINOWN} -g 
> ${BINGRP} -m ${BINMODE}' \
> + INSTALL_DATA='install -c -o ${BINOWN} -g ${DOCGRP} -m 
> ${NONBINMODE}' \

I think that should be ${DOCOWN} not ${BINOWN} for INSTALL_DATA,
otherwise OK natano@.


>   INSTALL_INFO_HOST_MODULES='${INSTALL_INFO_HOST_MODULES}' \
> install install-info
>  
> Index: gnu/usr.bin/binutils/Makefile.bsd-wrapper
> ===
> RCS file: /data/src/openbsd/src/gnu/usr.bin/binutils/Makefile.bsd-wrapper,v
> retrieving revision 1.83
> diff -u -p -r1.83 Makefile.bsd-wrapper
> --- gnu/usr.bin/binutils/Makefile.bsd-wrapper 1 Jun 2015 17:36:19 -   
> 1.83
> +++ gnu/usr.bin/binutils/Makefile.bsd-wrapper 11 Sep 2016 01:54:15 -
> @@ -81,7 +81,8 @@ install: maninstall
>   tooldir=${PREFIX} \
>   BSDSRCDIR=${BSDSRCDIR} \
>   INSTALL_MODULES='${INSTALL_MODULES}' \
> - INSTALL_PROGRAM='install -c -S' \
> + INSTALL_PROGRAM='install -c -S ${INSTALL_STRIP} -o ${BINOWN} -g 
> ${BINGRP} -m ${BINMODE}' \
> + INSTALL_DATA='install -c -o ${BINOWN} -g ${DOCGRP} -m 
> ${NONBINMODE}' \

same here


>   INSTALL_INFO_HOST_MODULES='${INSTALL_INFO_HOST_MODULES}' \
> install install-info
>  

Re: share/: install ownership fixes

[Martin Natano]

Another diff I typoed, also found by rpe@. Ok?

Index: share/misc/pcvtfonts/Makefile
===
RCS file: /cvs/src/share/misc/pcvtfonts/Makefile,v
retrieving revision 1.6
diff -u -p -r1.6 Makefile
--- share/misc/pcvtfonts/Makefile   13 May 2002 15:27:58 -  1.6
+++ share/misc/pcvtfonts/Makefile   8 Sep 2016 20:54:08 -
@@ -16,12 +16,9 @@ FONTDIR =${BINDIR}/misc/pcvtfonts
 all: $(FONTS)
 
 install: ${FONTS}
-   @if [ ! -d ${DESTDIR}${FONTDIR} ]; then mkdir ${DESTDIR}${FONTDIR};fi
-   @for i in ${FONTS}; do \
-   echo "installing font $$i into ${DESTDIR}${FONTDIR}"; \
-   install -c -m ${LIBMODE} -o ${LIBOWN} -g ${LIBGRP} \
-   $$i ${DESTDIR}${FONTDIR}; \
-   done
+   ${INSTALL} -d -o root -g wheel ${DESTDIR}${FONTDIR}
+   ${INSTALL} ${INSTALL_COPY} -m ${LIBMODE} -o ${LIBOWN} -g ${LIBGRP} \
+   ${FONTS} ${DESTDIR}${FONTDIR}
 
 clean:
rm -f ${CLEANFILES}
Index: share/snmp/Makefile
===
RCS file: /cvs/src/share/snmp/Makefile,v
retrieving revision 1.4
diff -u -p -r1.4 Makefile
--- share/snmp/Makefile 29 Jan 2016 03:06:00 -  1.4
+++ share/snmp/Makefile 8 Sep 2016 21:02:02 -
@@ -8,6 +8,7 @@ FILES+= OPENBSD-RELAYD-MIB.txt
 all clean cleandir depend lint obj tags: _SUBDIRUSE
 
 realinstall:
-   ${INSTALL} -c -m 0444 ${FILES} ${DESTDIR}${BINDIR}/snmp/mibs
+   ${INSTALL} ${INSTALL_COPY} -o root -g wheel -m 0444 \
+   ${FILES} ${DESTDIR}${BINDIR}/snmp/mibs
 
 .include 
Index: share/termtypes/Makefile
===
RCS file: /cvs/src/share/termtypes/Makefile,v
retrieving revision 1.24
diff -u -p -r1.24 Makefile
--- share/termtypes/Makefile3 Dec 2015 11:30:46 -   1.24
+++ share/termtypes/Makefile10 Sep 2016 20:11:58 -
@@ -14,12 +14,14 @@ termcap: termtypes.master
@[ -s ${.TARGET} ] || exit 1
 
 realinstall:
+   ${INSTALL} -d -o root -g wheel ${DESTDIR}${BINDIR}/terminfo
find terminfo -type f -exec \
 ${INSTALL} -D ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 444 \
 {} ${DESTDIR}${BINDIR}/{} \;
${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 444 termcap \
 ${DESTDIR}${BINDIR}/misc/termcap
ln -fs ${BINDIR}/misc/termcap ${DESTDIR}/etc/termcap
+   chown -h root:wheel ${DESTDIR}/etc/termcap
 
 clean:
rm -f termcap

etc ownership fixes

[Martin Natano]

Diff below sets the owner for the /etc/localtime, /etc/rmt, /var/tmp
and /sys symlinks and for the /var/sysmerge/etc.tgz tarball.

This is the last of the noperm related pending M's in my tree.

Ok?

natano


Index: etc/Makefile
===
RCS file: /cvs/src/etc/Makefile,v
retrieving revision 1.430
diff -u -p -r1.430 Makefile
--- etc/Makefile3 Sep 2016 13:37:40 -   1.430
+++ etc/Makefile10 Sep 2016 18:30:28 -
@@ -154,7 +154,9 @@ distribution-etc-root-var: distrib-dirs
${INSTALL} -c -o root -g wheel -m 644 *.pub \
${DESTDIR}/etc/signify
ln -fs ${TZDIR}/${LOCALTIME} ${DESTDIR}/etc/localtime
+   chown -h root:wheel ${DESTDIR}/etc/localtime
ln -fs /usr/sbin/rmt ${DESTDIR}/etc/rmt
+   chown -h root:wheel ${DESTDIR}/etc/rmt
${INSTALL} -c -o root -g wheel -m 644 minfree \
${DESTDIR}/var/crash
${INSTALL} -c -o ${BINOWN} -g operator -m 664 /dev/null \
@@ -206,6 +208,7 @@ distribution-etc-root-var: distrib-dirs
${INSTALL} -c -o ${BINOWN} -g ${BINGRP} -m 555 ${RCDAEMONS} \
${DESTDIR}/etc/rc.d
cd ${DESTDIR}/var; ln -fs ../tmp
+   chmod -h root:wheel ${DESTDIR}/var/tmp
touch ${DESTDIR}/var/sysmerge/etcsum
cd ${DESTDIR}/ && \
sort ${.CURDIR}/../distrib/sets/lists/etc/{mi,md.${MACHINE}} | \
@@ -213,6 +216,7 @@ distribution-etc-root-var: distrib-dirs
cd ${DESTDIR}/ && \
sort ${.CURDIR}/../distrib/sets/lists/etc/{mi,md.${MACHINE}} | \
pax -w -d | gzip -9 > ${DESTDIR}/var/sysmerge/etc.tgz
+   chown root:wheel ${DESTDIR}/var/sysmerge/etc.tgz
 
 distribution:
exec ${SUDO} ${MAKE} distribution-etc-root-var
@@ -227,6 +231,7 @@ distrib-dirs:
${INSTALL} -d -o root -g wsrc -m 775 ${DESTDIR}/usr/src; \
fi
cd ${DESTDIR}/; ln -fhs usr/src/sys sys
+   chmod -h root:wheel ${DESTDIR}/sys
 
 .ifndef RELEASEDIR
 release:

/usr/sbin/sysctl owner

[Martin Natano]

Yet another symlink that belongs to root. Ok?

natano


Index: sbin/sysctl/Makefile
===
RCS file: /cvs/src/sbin/sysctl/Makefile,v
retrieving revision 1.11
diff -u -p -r1.11 Makefile
--- sbin/sysctl/Makefile4 May 2016 19:48:08 -   1.11
+++ sbin/sysctl/Makefile10 Sep 2016 18:05:09 -
@@ -7,5 +7,6 @@ CPPFLAGS+=  -D_LIBKVM
 
 afterinstall:
ln -sf ../../sbin/sysctl ${DESTDIR}/usr/sbin
+   chown -h root:wheel ${DESTDIR}/usr/sbin/sysctl
 
 .include 

mailwrapper symlinks owner

[Martin Natano]

Another set of symlinks, same drill: the owner should be root. Ok?

natano


Index: usr.sbin/mailwrapper/Makefile
===
RCS file: /cvs/src/usr.sbin/mailwrapper/Makefile,v
retrieving revision 1.5
diff -u -p -r1.5 Makefile
--- usr.sbin/mailwrapper/Makefile   16 Mar 2009 22:34:13 -  1.5
+++ usr.sbin/mailwrapper/Makefile   10 Sep 2016 17:53:45 -
@@ -13,5 +13,9 @@ afterinstall:
ln -fs /usr/sbin/mailwrapper ${DESTDIR}/usr/sbin/makemap
ln -fs /usr/sbin/mailwrapper ${DESTDIR}/usr/bin/hoststat
ln -fs /usr/sbin/mailwrapper ${DESTDIR}/usr/bin/purgestat
+   chown -h root:wheel ${DESTDIR}/usr/sbin/sendmail \
+   ${DESTDIR}/usr/bin/newaliases ${DESTDIR}/usr/bin/mailq \
+   ${DESTDIR}/usr/sbin/makemap ${DESTDIR}/usr/bin/hoststat \
+   ${DESTDIR}/usr/bin/purgestat
 
 .include 

mg docs ownership

[Martin Natano]

We should be explicit about owner/group when using install, so this also
works correctly with noperm. Ok?

natano


Index: usr.bin/mg/Makefile
===
RCS file: /cvs/src/usr.bin/mg/Makefile,v
retrieving revision 1.31
diff -u -p -r1.31 Makefile
--- usr.bin/mg/Makefile 29 Sep 2015 03:50:58 -  1.31
+++ usr.bin/mg/Makefile 10 Sep 2016 17:45:11 -
@@ -24,8 +24,8 @@ SRCS= autoexec.c basic.c bell.c buffer.c
 SRCS+= cmode.c cscope.c dired.c grep.c tags.c theo.c
 
 afterinstall:
-   ${INSTALL} -d ${DESTDIR}${DOCDIR}/mg
-   ${INSTALL} -m ${DOCMODE} -c ${.CURDIR}/tutorial \
-   ${DESTDIR}${DOCDIR}/mg
+   ${INSTALL} -d -o root -g wheel ${DESTDIR}${DOCDIR}/mg
+   ${INSTALL} ${INSTALL_COPY} -o root -g wheel -m ${DOCMODE} \
+   ${.CURDIR}/tutorial ${DESTDIR}${DOCDIR}/mg
 
 .include 

create /usr/share/calendar/$lang with root owner

[Martin Natano]

Currently the /usr/share/calendar/$lang directories are created with the
build user as owner, but should be owned by root. Ok?

natano


Index: usr.bin/calendar/Makefile
===
RCS file: /cvs/src/usr.bin/calendar/Makefile,v
retrieving revision 1.10
diff -u -p -r1.10 Makefile
--- usr.bin/calendar/Makefile   23 Oct 2015 10:33:52 -  1.10
+++ usr.bin/calendar/Makefile   10 Sep 2016 17:37:28 -
@@ -8,11 +8,10 @@ beforeinstall:
${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 444 \
${.CURDIR}/calendars/calendar.* ${DESTDIR}/usr/share/calendar
 .for lang in ${INTER}
-   @test -d ${DESTDIR}/usr/share/calendar/${lang} || \
-   mkdir ${DESTDIR}/usr/share/calendar/${lang}
+   ${INSTALL} -d -o root -g wheel ${DESTDIR}/usr/share/calendar/${lang}
${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 444 \
${.CURDIR}/calendars/${lang}/calendar.* \
-   ${DESTDIR}/usr/share/calendar/${lang}; 
+   ${DESTDIR}/usr/share/calendar/${lang}
 .endfor
 
 .include 

Re: new getformat() for jot(1)

[Martin Natano]

On Sat, Sep 03, 2016 at 04:55:59PM +0100, Theo Buehler wrote:
> The -w flag to jot() allows the user to specify a format string that
> will be passed to printf after it was verified that it contains only one
> conversion specifier.  There are some subtle differences what jot's
> getformat() accepts and what printf will do. Thus, I took vfprintf.c and
> carved out whatever is unneeded for jot itself. The result is a slightly
> less ugly function in a separate file.

Please see some comments below.

I really tried to understand all the corner cases in the getformat()
function, but couldn't wrap my head around it. I believe it would be
best to just axe the -w flag. Yes, there are probably scripts out there
using it, but I think carrying that burden around is not worth it. Every
possible implementation either is an adapted reimplementation of printf,
or whitewashing the string before passing it to printf(). I would like
to remind of the patch(1) ed script issue that resulted in shell
injection, just because the whitewash code and the actual parser where
not in sync. It's a losing game.

natano


> 
> Index: Makefile
> ===
> RCS file: /var/cvs/src/usr.bin/jot/Makefile,v
> retrieving revision 1.5
> diff -u -p -r1.5 Makefile
> --- Makefile  10 Jan 2016 01:15:52 -  1.5
> +++ Makefile  3 Sep 2016 15:48:06 -
> @@ -1,6 +1,7 @@
>  #$OpenBSD: Makefile,v 1.5 2016/01/10 01:15:52 tb Exp $
>  
>  PROG=jot
> +SRCS=getformat.c jot.c
>  CFLAGS+= -Wall
>  LDADD+=  -lm
>  DPADD+=  ${LIBM}
> Index: getformat.c
> ===
> RCS file: getformat.c
> diff -N getformat.c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ getformat.c   3 Sep 2016 15:55:21 -
> @@ -0,0 +1,188 @@
> +/*   $OpenBSD$   */
> +/*-
> + * Copyright (c) 1990 The Regents of the University of California.
> + * All rights reserved.
> + *
> + * This code is derived from software contributed to Berkeley by
> + * Chris Torek.
> + *
> + * 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.
> + * 3. Neither the name of the University nor the names of its contributors
> + *may be used to endorse or promote products derived from this software
> + *without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE REGENTS 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 REGENTS 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.
> + */
> +
> +/* Based on src/lib/libc/stdio/vfprintf.c r1.77 */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +extern int   prec;
> +extern bool  boring;
> +extern bool  chardata;
> +extern bool  infinity;
> +extern bool  intdata;
> +extern bool  longdata;
> +extern bool  nosign;
> +extern bool  randomize;

'boring', 'infinity' and 'randomize' are unused in getformat.c.


> +
> +int getformat(char *);
> +
> +/*
> + * Macros for converting digits to letters and vice versa
> + */
> +#define  to_digit(c) ((c) - '0')
> +#define  is_digit(c) ((unsigned)to_digit(c) <= 9)

I would prefer this to be less magic. How about something like this
instead?

#define is_digit(c) ((c) >= '0' && (c) <= '9')

This would also allow to remove the to_digit() macro.


> +
> +int
> +getformat(char *fmt)
> +{
> + int ch; /* character from fmt */
> + wchar_t wc;
> + mbstate_t ps;
> + size_t sz;
> + bool firsttime = true;
> +
> + sz = sizeof(fmt) - strlen(fmt) - 1;

The sizeof() doesn't do what you expect it to do here. 'fmt' is just a
pointer here, so the value returned is far to low. What we want is the
size of the original array instead.

$ jot -w 'xxx' 10
jot: -w word too long


> +
> + memset(, 0, sizeof(ps));
> + /*
> +  * Scan the 

remove usermount remnants

[Martin Natano]

usermount is dead. Ok?

natano


Index: lib/libc/gen/sysctl.3
===
RCS file: /cvs/src/lib/libc/gen/sysctl.3,v
retrieving revision 1.267
diff -u -p -r1.267 sysctl.3
--- lib/libc/gen/sysctl.3   20 Jul 2016 09:15:28 -  1.267
+++ lib/libc/gen/sysctl.3   4 Sep 2016 21:11:14 -
@@ -474,7 +474,6 @@ information.
 .It Dv KERN_TIMECOUNTER Ta "node" Ta "not applicable"
 .It Dv KERN_TTY Ta "node" Ta "not applicable"
 .It Dv KERN_TTYCOUNT Ta "integer" Ta "no"
-.It Dv KERN_USERMOUNT Ta "integer" Ta "yes"
 .It Dv KERN_VERSION Ta "string" Ta "no"
 .It Dv KERN_WATCHDOG Ta "node" Ta "not applicable"
 .It Dv KERN_WXABORT Ta "integer" Ta "yes"
@@ -1016,8 +1015,6 @@ Returns the number of input characters i
 Number of available
 .Xr tty 4
 devices.
-.It Dv KERN_USERMOUNT
-Currently a no-op.
 .It Dv KERN_VERSION
 The system version string.
 .It Dv KERN_WATCHDOG
Index: sbin/sysctl/sysctl.8
===
RCS file: /cvs/src/sbin/sysctl/sysctl.8,v
retrieving revision 1.204
diff -u -p -r1.204 sysctl.8
--- sbin/sysctl/sysctl.827 Jul 2016 20:51:46 -  1.204
+++ sbin/sysctl/sysctl.84 Sep 2016 21:04:44 -
@@ -139,7 +139,6 @@ and a few require a kernel compiled with
 .It kern.osversion Ta string Ta no
 .It kern.somaxconn Ta integer Ta yes
 .It kern.sominconn Ta integer Ta yes
-.It kern.usermount Ta integer Ta yes
 .It kern.nosuidcoredump Ta integer Ta yes
 .It kern.fsync Ta integer Ta no
 .It kern.sysvmsg Ta integer Ta no
Index: sys/isofs/cd9660/cd9660_vfsops.c
===
RCS file: /cvs/src/sys/isofs/cd9660/cd9660_vfsops.c,v
retrieving revision 1.82
diff -u -p -r1.82 cd9660_vfsops.c
--- sys/isofs/cd9660/cd9660_vfsops.c2 Sep 2016 10:16:03 -   1.82
+++ sys/isofs/cd9660/cd9660_vfsops.c4 Sep 2016 20:02:52 -
@@ -179,19 +179,6 @@ cd9660_mount(mp, path, data, ndp, p)
return (ENXIO);
}
 
-   /*
-* If mount by non-root, then verify that user has necessary
-* permissions on the device.
-*/
-   if (suser(p, 0) != 0) {
-   vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY, p);
-   error = VOP_ACCESS(devvp, VREAD, p->p_ucred, p);
-   if (error) {
-   vput(devvp);
-   return (error);
-   }
-   VOP_UNLOCK(devvp, p);
-   }
if ((mp->mnt_flag & MNT_UPDATE) == 0)
error = iso_mountfs(devvp, mp, p, );
else {
Index: sys/isofs/udf/udf_vfsops.c
===
RCS file: /cvs/src/sys/isofs/udf/udf_vfsops.c,v
retrieving revision 1.54
diff -u -p -r1.54 udf_vfsops.c
--- sys/isofs/udf/udf_vfsops.c  25 Aug 2016 00:06:44 -  1.54
+++ sys/isofs/udf/udf_vfsops.c  4 Sep 2016 20:04:57 -
@@ -170,17 +170,6 @@ udf_mount(struct mount *mp, const char *
return (ENXIO);
}
 
-   /* Check the access rights on the mount device */
-   if (p->p_ucred->cr_uid) {
-   vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY, p);
-   error = VOP_ACCESS(devvp, VREAD, p->p_ucred, p);
-   VOP_UNLOCK(devvp, p);
-   if (error) {
-   vrele(devvp);
-   return (error);
-   }
-   }
-
if ((error = udf_mountfs(devvp, mp, args.lastblock, p))) {
vrele(devvp);
return (error);
Index: sys/kern/kern_sysctl.c
===
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.308
diff -u -p -r1.308 kern_sysctl.c
--- sys/kern/kern_sysctl.c  4 Sep 2016 09:22:29 -   1.308
+++ sys/kern/kern_sysctl.c  4 Sep 2016 21:07:46 -
@@ -414,10 +414,6 @@ kern_sysctl(int *name, u_int namelen, vo
return (sysctl_int(oldp, oldlenp, newp, newlen, ));
case KERN_SOMINCONN:
return (sysctl_int(oldp, oldlenp, newp, newlen, ));
-   case KERN_USERMOUNT: {
-   int usermount = 0;
-   return (sysctl_rdint(oldp, oldlenp, newp, usermount));
-   }
case KERN_ARND: {
char buf[512];
 
Index: sys/kern/vfs_syscalls.c
===
RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.263
diff -u -p -r1.263 vfs_syscalls.c
--- sys/kern/vfs_syscalls.c 14 Jul 2016 15:39:40 -  1.263
+++ sys/kern/vfs_syscalls.c 4 Sep 2016 20:28:57 -
@@ -110,7 +110,6 @@ sys_mount(struct proc *p, void *v, regis
int error, mntflag = 0;
char fstypename[MFSNAMELEN];
char fspath[MNAMELEN];
-   struct vattr va;
struct nameidata nd;
struct vfsconf *vfsp;
int flags = SCARG(uap, flags);
@@ -150,29 +149,6 @@ sys_mount(struct 

regen pkg-config files

[Martin Natano]

Currently we regenerate the pkg-config files on every install. Following
patch allows to only regen the files when the library version changed.

Ok?

natano


Index: lib/libcrypto/Makefile
===
RCS file: /cvs/src/lib/libcrypto/Makefile,v
retrieving revision 1.2
diff -u -p -r1.2 Makefile
--- lib/libcrypto/Makefile  3 Sep 2016 12:42:46 -   1.2
+++ lib/libcrypto/Makefile  3 Sep 2016 21:13:00 -
@@ -438,12 +438,12 @@ distribution:
${INSTALL} ${INSTALL_COPY} -g ${BINGRP} -m 444 \
   ${.CURDIR}/x509v3.cnf ${DESTDIR}/etc/ssl/x509v3.cnf
 
-beforeinstall:
+${PC_FILES}: opensslv.h
/bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
-.for p in ${PC_FILES}
+
+beforeinstall: ${PC_FILES}
${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
-   -m ${SHAREMODE} ${.OBJDIR}/$p ${DESTDIR}/usr/lib/pkgconfig/
-.endfor
+   -m ${SHAREMODE} ${.OBJDIR}/${PC_FILES} ${DESTDIR}/usr/lib/pkgconfig/
 
 .include 
 .include 
Index: lib/libexpat/Makefile
===
RCS file: /cvs/src/lib/libexpat/Makefile,v
retrieving revision 1.9
diff -u -p -r1.9 Makefile
--- lib/libexpat/Makefile   2 Aug 2012 13:38:38 -   1.9
+++ lib/libexpat/Makefile   3 Sep 2016 21:08:37 -
@@ -17,8 +17,10 @@ includes:
  ${INSTALL} ${INSTALL_COPY} -m 444 -o $(BINOWN) -g $(BINGRP) \
  ${.CURDIR}/lib/expat_external.h 
${DESTDIR}/usr/include/expat_external.h
 
-beforeinstall:
+${PC_FILES}: lib/expat.h
/bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
+
+beforeinstall: ${PC_FILES}
${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
-m ${SHAREMODE} ${.OBJDIR}/${PC_FILES} ${DESTDIR}/usr/lib/pkgconfig/
 
Index: lib/libfuse/Makefile
===
RCS file: /cvs/src/lib/libfuse/Makefile,v
retrieving revision 1.8
diff -u -p -r1.8 Makefile
--- lib/libfuse/Makefile30 Mar 2016 06:38:42 -  1.8
+++ lib/libfuse/Makefile3 Sep 2016 21:08:57 -
@@ -29,8 +29,10 @@ includes:
eval "$$j"; \
done
 
-beforeinstall:
+${PC_FILES}: fuse_private.h
/bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
+
+beforeinstall: ${PC_FILES}
${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
-m ${SHAREMODE} ${.OBJDIR}/${PC_FILES} ${DESTDIR}/usr/lib/pkgconfig/
 
Index: lib/libssl/Makefile
===
RCS file: /cvs/src/lib/libssl/Makefile,v
retrieving revision 1.20
diff -u -p -r1.20 Makefile
--- lib/libssl/Makefile 3 Sep 2016 12:42:42 -   1.20
+++ lib/libssl/Makefile 3 Sep 2016 19:42:32 -
@@ -1,4 +1,4 @@
-# $OpenBSD: Makefile,v 1.20 2016/09/03 12:42:42 beck Exp $
+# $OpenBSD: Makefile,v 1.20 2016/09/02 17:00:14 beck Exp $
 
 SUBDIR= man
 PC_FILES=openssl.pc libssl.pc
@@ -48,10 +48,12 @@ includes:
 
 .include 
 
-beforeinstall:
+${PC_FILES}: ${.CURDIR}/../libcrypto/opensslv.h
+   /bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
+
+beforeinstall: ${PC_FILES}
nm -o lib${LIB}.a | egrep -w 'printf|fprintf' && \
(echo please fix stdio usage in this library; false) || true
-   /bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
 .for p in ${PC_FILES}
${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
-m ${SHAREMODE} ${.OBJDIR}/$p ${DESTDIR}/usr/lib/pkgconfig/
Index: lib/libz/Makefile
===
RCS file: /cvs/src/lib/libz/Makefile,v
retrieving revision 1.18
diff -u -p -r1.18 Makefile
--- lib/libz/Makefile   30 Mar 2016 06:38:43 -  1.18
+++ lib/libz/Makefile   3 Sep 2016 21:09:33 -
@@ -19,8 +19,10 @@ includes:
eval "$$j"; \
done
 
-beforeinstall:
+${PC_FILES}: zlib.h
/bin/sh ${.CURDIR}/generate_pkgconfig.sh -c ${.CURDIR} -o ${.OBJDIR}
+
+beforeinstall: ${PC_FILES}
${INSTALL} ${INSTALL_COPY} -o root -g ${SHAREGRP} \
-m ${SHAREMODE} ${.OBJDIR}/${PC_FILES} ${DESTDIR}/usr/lib/pkgconfig/
 

Re: iwm(4): Use htole16 for policy field in iwm_time_event_cmd_v2 struct.

[Martin Natano]

On Wed, Aug 31, 2016 at 09:39:47PM +0200, Imre Vadasz wrote:
> Hi,
> The policy field in struct iwm_time_event_cmd_v2 is just a 16bit integer,
> so we should use htole16() when setting it, instead of htole32().

Makes sense to me, although I don't have hardware to test this.


> 
> Index: sys/dev/pci/if_iwm.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
> retrieving revision 1.99
> diff -u -r1.99 if_iwm.c
> --- sys/dev/pci/if_iwm.c  17 Aug 2016 09:39:38 -  1.99
> +++ sys/dev/pci/if_iwm.c  31 Aug 2016 19:37:05 -
> @@ -2379,7 +2379,7 @@
>   time_cmd.duration = htole32(duration);
>   time_cmd.repeat = 1;
>   time_cmd.policy
> - = htole32(IWM_TE_V2_NOTIF_HOST_EVENT_START |
> + = htole16(IWM_TE_V2_NOTIF_HOST_EVENT_START |
>   IWM_TE_V2_NOTIF_HOST_EVENT_END |
>   IWM_T2_V2_START_IMMEDIATELY);
>  
> 

Re: remove ntfs write code

[Martin Natano]

On Wed, Aug 31, 2016 at 05:17:41PM -0600, Bob Beck wrote:
> Yes, ok beck@
> 
> to be shortly followed by the ntfs code - don't we have a fuse version of
> this?

There's the ntfs_3g port benno uses. Iirc, access via ntfs_3g is
somewhat slower than with the native filesystem (not that I care).


> 
> 
> On Wed, Aug 31, 2016 at 3:34 PM, Martin Natano <nat...@natano.net> wrote:
> 
> > mount_ntfs forces the mount point to be MNT_RDONLY, so the write parts
> > in ntfs are never used. OK to remove?
> >
> > natano
> >
> >
> > Index: ntfs/ntfs_subr.c
> > ===
> > RCS file: /cvs/src/sys/ntfs/ntfs_subr.c,v
> > retrieving revision 1.47
> > diff -u -p -r1.47 ntfs_subr.c
> > --- ntfs/ntfs_subr.c31 Aug 2016 15:13:57 -  1.47
> > +++ ntfs/ntfs_subr.c31 Aug 2016 19:58:31 -
> > @@ -1336,152 +1336,6 @@ ntfs_filesize(struct ntfsmount *ntmp, st
> >  }
> >
> >  /*
> > - * This is one of the write routines.
> > - */
> > -int
> > -ntfs_writeattr_plain(struct ntfsmount *ntmp, struct ntnode *ip,
> > -u_int32_t attrnum, char *attrname, off_t roff, size_t rsize, void
> > *rdata,
> > -size_t *initp, struct uio *uio)
> > -{
> > -   size_t  init;
> > -   int error = 0;
> > -   off_t   off = roff;
> > -   size_t  left = rsize, towrite;
> > -   caddr_t data = rdata;
> > -   struct ntvattr *vap;
> > -   *initp = 0;
> > -
> > -   while (left) {
> > -   error = ntfs_ntvattrget(ntmp, ip, attrnum, attrname,
> > -   ntfs_btocn(off), );
> > -   if (error)
> > -   return (error);
> > -   towrite = MIN(left, ntfs_cntob(vap->va_vcnend + 1) - off);
> > -   DDPRINTF("ntfs_writeattr_plain: o: %lld, s: %zu "
> > -   "(%llu - %llu)\n", off, towrite,
> > -   vap->va_vcnstart, vap->va_vcnend);
> > -   error = ntfs_writentvattr_plain(ntmp, ip, vap,
> > -off -
> > ntfs_cntob(vap->va_vcnstart),
> > -towrite, data, , uio);
> > -   if (error) {
> > -   DPRINTF("ntfs_writeattr_plain:
> > ntfs_writentvattr_plain "
> > -   "failed: o: %lld, s: %zu\n", off, towrite);
> > -   DPRINTF("ntfs_writeattr_plain: attrib: %llu -
> > %llu\n",
> > -   vap->va_vcnstart, vap->va_vcnend);
> > -   ntfs_ntvattrrele(vap);
> > -   break;
> > -   }
> > -   ntfs_ntvattrrele(vap);
> > -   left -= towrite;
> > -   off += towrite;
> > -   data = data + towrite;
> > -   *initp += init;
> > -   }
> > -
> > -   return (error);
> > -}
> > -
> > -/*
> > - * This is one of the write routines.
> > - *
> > - * ntnode should be locked.
> > - */
> > -int
> > -ntfs_writentvattr_plain(struct ntfsmount *ntmp, struct ntnode *ip,
> > -struct ntvattr *vap, off_t roff, size_t rsize, void *rdata, size_t
> > *initp,
> > -struct uio *uio)
> > -{
> > -   int error = 0;
> > -   off_t   off;
> > -   int cnt;
> > -   cn_tccn, ccl, cn, cl;
> > -   caddr_t data = rdata;
> > -   struct buf *bp;
> > -   size_t  left, tocopy;
> > -
> > -   *initp = 0;
> > -
> > -   if ((vap->va_flag & NTFS_AF_INRUN) == 0) {
> > -   DPRINTF("ntfs_writevattr_plain: CAN'T WRITE RES.
> > ATTRIBUTE\n");
> > -   return ENOTTY;
> > -   }
> > -
> > -   DDPRINTF("ntfs_writentvattr_plain: data in run: %lu chains\n",
> > -   vap->va_vruncnt);
> > -
> > -   off = roff;
> > -   left = rsize;
> > -
> > -   for (cnt = 0; left && (cnt < vap->va_vruncnt); cnt++) {
> > -   ccn = vap->va_vruncn[cnt];
> > -   ccl = vap->va_vruncl[cnt];
> > -
> > -   DDPRINTF("ntfs_writentvattr_plain: left %zu, cn: 0x%llx, "
> > -   "cl: %llu, off: %lld\n

remove ntfs write code

[Martin Natano]

mount_ntfs forces the mount point to be MNT_RDONLY, so the write parts
in ntfs are never used. OK to remove?

natano


Index: ntfs/ntfs_subr.c
===
RCS file: /cvs/src/sys/ntfs/ntfs_subr.c,v
retrieving revision 1.47
diff -u -p -r1.47 ntfs_subr.c
--- ntfs/ntfs_subr.c31 Aug 2016 15:13:57 -  1.47
+++ ntfs/ntfs_subr.c31 Aug 2016 19:58:31 -
@@ -1336,152 +1336,6 @@ ntfs_filesize(struct ntfsmount *ntmp, st
 }
 
 /*
- * This is one of the write routines.
- */
-int
-ntfs_writeattr_plain(struct ntfsmount *ntmp, struct ntnode *ip,
-u_int32_t attrnum, char *attrname, off_t roff, size_t rsize, void *rdata,
-size_t *initp, struct uio *uio)
-{
-   size_t  init;
-   int error = 0;
-   off_t   off = roff;
-   size_t  left = rsize, towrite;
-   caddr_t data = rdata;
-   struct ntvattr *vap;
-   *initp = 0;
-
-   while (left) {
-   error = ntfs_ntvattrget(ntmp, ip, attrnum, attrname,
-   ntfs_btocn(off), );
-   if (error)
-   return (error);
-   towrite = MIN(left, ntfs_cntob(vap->va_vcnend + 1) - off);
-   DDPRINTF("ntfs_writeattr_plain: o: %lld, s: %zu "
-   "(%llu - %llu)\n", off, towrite,
-   vap->va_vcnstart, vap->va_vcnend);
-   error = ntfs_writentvattr_plain(ntmp, ip, vap,
-off - ntfs_cntob(vap->va_vcnstart),
-towrite, data, , uio);
-   if (error) {
-   DPRINTF("ntfs_writeattr_plain: ntfs_writentvattr_plain "
-   "failed: o: %lld, s: %zu\n", off, towrite);
-   DPRINTF("ntfs_writeattr_plain: attrib: %llu - %llu\n",
-   vap->va_vcnstart, vap->va_vcnend);
-   ntfs_ntvattrrele(vap);
-   break;
-   }
-   ntfs_ntvattrrele(vap);
-   left -= towrite;
-   off += towrite;
-   data = data + towrite;
-   *initp += init;
-   }
-
-   return (error);
-}
-
-/*
- * This is one of the write routines.
- *
- * ntnode should be locked.
- */
-int
-ntfs_writentvattr_plain(struct ntfsmount *ntmp, struct ntnode *ip,
-struct ntvattr *vap, off_t roff, size_t rsize, void *rdata, size_t *initp,
-struct uio *uio)
-{
-   int error = 0;
-   off_t   off;
-   int cnt;
-   cn_tccn, ccl, cn, cl;
-   caddr_t data = rdata;
-   struct buf *bp;
-   size_t  left, tocopy;
-
-   *initp = 0;
-
-   if ((vap->va_flag & NTFS_AF_INRUN) == 0) {
-   DPRINTF("ntfs_writevattr_plain: CAN'T WRITE RES. ATTRIBUTE\n");
-   return ENOTTY;
-   }
-
-   DDPRINTF("ntfs_writentvattr_plain: data in run: %lu chains\n",
-   vap->va_vruncnt);
-
-   off = roff;
-   left = rsize;
-
-   for (cnt = 0; left && (cnt < vap->va_vruncnt); cnt++) {
-   ccn = vap->va_vruncn[cnt];
-   ccl = vap->va_vruncl[cnt];
-
-   DDPRINTF("ntfs_writentvattr_plain: left %zu, cn: 0x%llx, "
-   "cl: %llu, off: %lld\n", left, ccn, ccl, off);
-
-   if (ntfs_cntob(ccl) < off) {
-   off -= ntfs_cntob(ccl);
-   cnt++;
-   continue;
-   }
-   if (!ccn && ip->i_number != NTFS_BOOTINO)
-   continue; /* XXX */
-
-   ccl -= ntfs_btocn(off);
-   cn = ccn + ntfs_btocn(off);
-   off = ntfs_btocnoff(off);
-
-   while (left && ccl) {
-   /*
-* Always read and write single clusters at a time -
-* we need to avoid requesting differently-sized
-* blocks at the same disk offsets to avoid
-* confusing the buffer cache.
-*/
-   tocopy = MIN(left, ntfs_cntob(1) - off);
-   cl = ntfs_btocl(tocopy + off);
-   KASSERT(cl == 1 && tocopy <= ntfs_cntob(1));
-   DDPRINTF("ntfs_writentvattr_plain: write: cn: 0x%llx "
-   "cl: %llu, off: %lld len: %zu, left: %zu\n",
-   cn, cl, off, tocopy, left);
-   if ((off == 0) && (tocopy == ntfs_cntob(cl)))
-   {
-   bp = getblk(ntmp->ntm_devvp, ntfs_cntobn(cn),
-   ntfs_cntob(cl), 0, 0);
-   clrbuf(bp);
-   } else {
-   error = bread(ntmp->ntm_devvp, ntfs_cntobn(cn),
-  

fusefs lookup/reclaim bug

[Martin Natano]

Watch this:

$ doas fuse-zip ~/Downloads/foo.zip /mnt
$ ls /mnt
openbsd-www
$ grep -IR foo /usr/src > /dev/null # force vfs to reclaim vnodes
$ ls /mnt
ls: /mnt: No such file or directory
$

Currently fusefs nodes in the kernel remember the parent inode number
for ".." lookups. This only works until the kernel starts to reuse
vnodes and the parent's vnode is reclaimed and the ino to path mapping
is removed from the userland process by libfuse. I suggest to fix this
by using reference counting in libfuse, so that parent mapping are
retained as long as a child uses them. Also, the root node should never
be freed.

Ok?

natano


Index: lib/libfuse/fuse_ops.c
===
RCS file: /cvs/src/lib/libfuse/fuse_ops.c,v
retrieving revision 1.25
diff -u -p -r1.25 fuse_ops.c
--- lib/libfuse/fuse_ops.c  30 Aug 2016 16:45:54 -  1.25
+++ lib/libfuse/fuse_ops.c  31 Aug 2016 10:14:58 -
@@ -432,15 +432,28 @@ ifuse_ops_lookup(struct fuse *f, struct 
DPRINTF("Inode:\t%llu\n", (unsigned long long)fbuf->fb_ino);
DPRINTF("For file %s\n", fbuf->fb_dat);
 
-   vn = get_vn_by_name_and_parent(f, fbuf->fb_dat, fbuf->fb_ino);
-   if (vn == NULL) {
-   vn = alloc_vn(f, (const char *)fbuf->fb_dat, -1, fbuf->fb_ino);
-   if (vn == NULL) {
-   fbuf->fb_err = -errno;
-   free(fbuf->fb_dat);
+   if (strcmp((const char *)fbuf->fb_dat, "..") == 0) {
+   vn = tree_get(>vnode_tree, fbuf->fb_ino);
+   if (vn == NULL || vn->parent == NULL) {
+   fbuf->fb_err = -ENOENT;
return (0);
}
-   set_vn(f, vn); /*XXX*/
+   vn = vn->parent;
+   if (vn->ino != FUSE_ROOT_INO)
+   ref_vn(vn);
+   } else {
+   vn = get_vn_by_name_and_parent(f, fbuf->fb_dat, fbuf->fb_ino);
+   if (vn == NULL) {
+   vn = alloc_vn(f, (const char *)fbuf->fb_dat, -1,
+   fbuf->fb_ino);
+   if (vn == NULL) {
+   fbuf->fb_err = -errno;
+   free(fbuf->fb_dat);
+   return (0);
+   }
+   set_vn(f, vn); /*XXX*/
+   } else if (vn->ino != FUSE_ROOT_INO)
+   ref_vn(vn);
}
 
DPRINTF("new ino %llu\n", (unsigned long long)vn->ino);
@@ -991,11 +1004,9 @@ ifuse_ops_reclaim(struct fuse *f, struct
 {
struct fuse_vnode *vn;
 
-   vn = tree_pop(>vnode_tree, fbuf->fb_ino);
-   if (vn) {
-   remove_vnode_from_name_tree(f, vn);
-   free(vn);
-   }
+   vn = tree_get(>vnode_tree, fbuf->fb_ino);
+   if (vn != NULL)
+   unref_vn(f, vn);
 
return (0);
 }
Index: lib/libfuse/fuse_private.h
===
RCS file: /cvs/src/lib/libfuse/fuse_private.h,v
retrieving revision 1.13
diff -u -p -r1.13 fuse_private.h
--- lib/libfuse/fuse_private.h  30 Aug 2016 16:45:54 -  1.13
+++ lib/libfuse/fuse_private.h  30 Aug 2016 17:06:27 -
@@ -34,7 +34,8 @@ struct fuse_args;
 
 struct fuse_vnode {
ino_t ino;
-   ino_t parent;
+   struct fuse_vnode *parent;
+   unsigned int ref;
 
char path[NAME_MAX + 1];
struct fuse_dirhandle *fd;
@@ -99,6 +100,8 @@ int  ifuse_exec_opcode(struct fuse *, str
 
 /* fuse_subr.c */
 struct fuse_vnode  *alloc_vn(struct fuse *, const char *, ino_t, ino_t);
+voidref_vn(struct fuse_vnode *);
+voidunref_vn(struct fuse *, struct fuse_vnode *);
 struct fuse_vnode  *get_vn_by_name_and_parent(struct fuse *, uint8_t *,
 ino_t);
 void   remove_vnode_from_name_tree(struct fuse *,
Index: lib/libfuse/fuse_subr.c
===
RCS file: /cvs/src/lib/libfuse/fuse_subr.c,v
retrieving revision 1.10
diff -u -p -r1.10 fuse_subr.c
--- lib/libfuse/fuse_subr.c 24 May 2016 19:24:46 -  1.10
+++ lib/libfuse/fuse_subr.c 31 Aug 2016 10:15:36 -
@@ -24,7 +24,7 @@
 #include "debug.h"
 
 struct fuse_vnode *
-alloc_vn(struct fuse *f, const char *path, ino_t ino, ino_t parent)
+alloc_vn(struct fuse *f, const char *path, ino_t ino, ino_t pino)
 {
struct fuse_vnode *vn;
 
@@ -34,13 +34,26 @@ alloc_vn(struct fuse *f, const char *pat
}
 
vn->ino = ino;
-   vn->parent = parent;
+   vn->ref = 1;
if (strlcpy(vn->path, path, sizeof(vn->path)) >= sizeof(vn->path)) {
DPRINTF("%s: strlcpy name too long\n", __func__);
free(vn);
return (NULL);
}
 
+   if (pino == (ino_t)0)
+   vn->parent = NULL;
+   else {
+   if ((vn->parent = 

replace struct vattr with struct stat in fusebuf

[Martin Natano]

In struct fusebuf the attributes (used for getattr/setattr and to return
the ino/mode of a newly created file) are stored in a struct vattr. The
problem with this is, that using struct vattr in userspace is suboptimal
as some related helpers are not available, e.g. VATTR_NULL() and IFTOVT().

What happens on getattr is this: fusefs requests the attributes for a
file from the userland process. libfuse calls the getattr fuse
operation, which fills in a struct stat. This struct is than converted
to a partial struct vattr, which is passed back to the kernel where
"fixup" takes place.

Following diff simplifies that process by replacing the struct vattr in
fusebuf with a struct stat. The conversion is moved to the kernel where
it belongs. As a side effect the  include can be removed
from libfuse.

Ok? Objections?

natano


Index: lib/libfuse/fuse_ops.c
===
RCS file: /cvs/src/lib/libfuse/fuse_ops.c,v
retrieving revision 1.24
diff -u -p -r1.24 fuse_ops.c
--- lib/libfuse/fuse_ops.c  5 Feb 2014 20:13:58 -   1.24
+++ lib/libfuse/fuse_ops.c  22 Aug 2016 20:53:46 -
@@ -30,53 +30,30 @@
return (0); \
}
 
-static void
-stat2attr(struct vattr *v, struct stat *st)
-{
-   v->va_fileid = st->st_ino;
-   v->va_bytes = st->st_blocks;
-   v->va_mode = st->st_mode;
-   v->va_nlink = st->st_nlink;
-   v->va_uid = st->st_uid;
-   v->va_gid = st->st_gid;
-   v->va_rdev = st->st_rdev;
-   v->va_size = st->st_size;
-   v->va_blocksize = st->st_blksize;
-   v->va_atime.tv_sec = st->st_atime;
-   v->va_atime.tv_nsec = st->st_atimensec;
-   v->va_mtime.tv_sec = st->st_mtime;
-   v->va_mtime.tv_nsec = st->st_mtimensec;
-   v->va_ctime.tv_sec = st->st_ctime;
-   v->va_ctime.tv_nsec = st->st_ctimensec;
-}
-
 static int
-update_vattr(struct fuse *f, struct vattr *attr, const char *realname,
+update_attr(struct fuse *f, struct stat *attr, const char *realname,
 struct fuse_vnode *vn)
 {
-   struct stat st;
int ret;
 
-   bzero(, sizeof(st));
-   ret = f->op.getattr(realname, );
+   memset(attr, 0, sizeof(struct stat));
+   ret = f->op.getattr(realname, attr);
 
-   if (st.st_blksize == 0)
-   st.st_blksize = 512;
-   if (st.st_blocks == 0)
-   st.st_blocks = 4;
+   if (attr->st_blksize == 0)
+   attr->st_blksize = 512;
+   if (attr->st_blocks == 0)
+   attr->st_blocks = 4;
 
-   st.st_ino = vn->ino;
+   attr->st_ino = vn->ino;
 
if (f->conf.set_mode)
-   st.st_mode = (st.st_mode & S_IFMT) | (0777 & ~f->conf.umask);
+   attr->st_mode = (attr->st_mode & S_IFMT) | (0777 & 
~f->conf.umask);
 
if (f->conf.set_uid)
-   st.st_uid = f->conf.uid;
+   attr->st_uid = f->conf.uid;
 
if (f->conf.set_gid)
-   st.st_gid = f->conf.gid;
-
-   stat2attr(attr, );
+   attr->st_gid = f->conf.gid;
 
return (ret);
 }
@@ -107,7 +84,7 @@ ifuse_ops_getattr(struct fuse *f, struct
DPRINTF("Opcode:\tgetattr\n");
DPRINTF("Inode:\t%llu\n", (unsigned long long)fbuf->fb_ino);
 
-   bzero(>fb_vattr, sizeof(fbuf->fb_vattr));
+   memset(>fb_attr, 0, sizeof(struct stat));
 
vn = tree_get(>vnode_tree, fbuf->fb_ino);
if (vn == NULL) {
@@ -121,7 +98,7 @@ ifuse_ops_getattr(struct fuse *f, struct
return (0);
}
 
-   fbuf->fb_err = update_vattr(f, >fb_vattr, realname, vn);
+   fbuf->fb_err = update_attr(f, >fb_attr, realname, vn);
free(realname);
 
return (0);
@@ -474,7 +451,7 @@ ifuse_ops_lookup(struct fuse *f, struct 
return (0);
}
 
-   fbuf->fb_err = update_vattr(f, >fb_vattr, realname, vn);
+   fbuf->fb_err = update_attr(f, >fb_attr, realname, vn);
free(fbuf->fb_dat);
free(realname);
 
@@ -614,9 +591,9 @@ ifuse_ops_create(struct fuse *f, struct 
fbuf->fb_err = -ENOSYS;
 
if (!fbuf->fb_err) {
-   fbuf->fb_err = update_vattr(f, >fb_vattr, realname, vn);
-   fbuf->fb_ino = fbuf->fb_vattr.va_fileid;
-   fbuf->fb_io_mode = fbuf->fb_vattr.va_mode;
+   fbuf->fb_err = update_attr(f, >fb_attr, realname, vn);
+   fbuf->fb_ino = fbuf->fb_attr.st_ino;
+   fbuf->fb_io_mode = fbuf->fb_attr.st_mode;
}
free(realname);
 
@@ -650,8 +627,8 @@ ifuse_ops_mkdir(struct fuse *f, struct f
fbuf->fb_err = f->op.mkdir(realname, mode);
 
if (!fbuf->fb_err) {
-   fbuf->fb_err = update_vattr(f, >fb_vattr, realname, vn);
-   fbuf->fb_io_mode = fbuf->fb_vattr.va_mode;
+   fbuf->fb_err = update_attr(f, >fb_attr, realname, vn);
+   fbuf->fb_io_mode = 

fusefs: remove update_vattr() helper function

[Martin Natano]

There are three callers of update_vattr(). Two of them don't use the
updated struct vattr afterwards, so the call can be removed. Following
diff removes both calls and the function itself, inlining the last
remaining call.

While there I removed this gem:

#if (S_BLKSIZE == 512)
v->va_bytes = v->va_bytes << 9;
#else
v->va_bytes = v->va_bytes * S_BLKSIZE;
#endif

Our compiler is mature enough to figure that out by itself. The checksum
of the generated object file doesn't change regardless which branch is
compiled in. Even if that wasn't the case, that's not what's gonna make
fuse slow.

Ok? Objections?

natano


Index: miscfs/fuse/fuse_lookup.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_lookup.c,v
retrieving revision 1.13
diff -u -p -r1.13 fuse_lookup.c
--- miscfs/fuse/fuse_lookup.c   16 Aug 2016 21:32:58 -  1.13
+++ miscfs/fuse/fuse_lookup.c   17 Aug 2016 20:40:38 -
@@ -174,14 +174,10 @@ fusefs_lookup(void *v)
error = 0;
} else {
error = VFS_VGET(fmp->mp, nid, );
-
-   if (!error)
-   tdp->v_type = IFTOVT(fbuf->fb_vattr.va_mode);
-
-   update_vattr(fmp->mp, >fb_vattr);
-
if (error)
goto out;
+
+   tdp->v_type = IFTOVT(fbuf->fb_vattr.va_mode);
 
if (vdp != NULL && vdp->v_type == VDIR)
VTOI(tdp)->parent = dp->ufs_ino.i_number;
Index: miscfs/fuse/fuse_vnops.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_vnops.c,v
retrieving revision 1.30
diff -u -p -r1.30 fuse_vnops.c
--- miscfs/fuse/fuse_vnops.c16 Aug 2016 21:32:58 -  1.30
+++ miscfs/fuse/fuse_vnops.c17 Aug 2016 20:40:38 -
@@ -203,19 +203,6 @@ filt_fusefsvnode(struct knote *kn, long 
return (kn->kn_fflags != 0);
 }
 
-void
-update_vattr(struct mount *mp, struct vattr *v)
-{
-   v->va_fsid = mp->mnt_stat.f_fsid.val[0];
-   v->va_type = IFTOVT(v->va_mode);
-#if (S_BLKSIZE == 512)
-   v->va_bytes = v->va_bytes << 9;
-#else
-   v->va_bytes = v->va_bytes * S_BLKSIZE;
-#endif
-   v->va_mode = v->va_mode & ~S_IFMT;
-}
-
 int
 fusefs_open(void *v)
 {
@@ -401,8 +388,13 @@ fusefs_getattr(void *v)
return (error);
}
 
-   update_vattr(fmp->mp, >fb_vattr);
memcpy(vap, >fb_vattr, sizeof(*vap));
+
+   vap->va_fsid = fmp->mp->mnt_stat.f_fsid.val[0];
+   vap->va_type = IFTOVT(vap->va_mode);
+   vap->va_bytes *= S_BLKSIZE;
+   vap->va_mode &= ~S_IFMT;
+
fb_delete(fbuf);
return (error);
 }
@@ -518,15 +510,12 @@ fusefs_setattr(void *v)
}
 
error = fb_queue(fmp->dev, fbuf);
-
if (error) {
if (error == ENOSYS)
fmp->undef_op |= UNDEF_SETATTR;
goto out;
}
 
-   update_vattr(fmp->mp, >fb_vattr);
-   memcpy(vap, >fb_vattr, sizeof(*vap));
VN_KNOTE(ap->a_vp, NOTE_ATTRIB);
 
 out:
Index: miscfs/fuse/fusefs.h
===
RCS file: /cvs/src/sys/miscfs/fuse/fusefs.h,v
retrieving revision 1.7
diff -u -p -r1.7 fusefs.h
--- miscfs/fuse/fusefs.h20 May 2014 13:32:22 -  1.7
+++ miscfs/fuse/fusefs.h17 Aug 2016 20:40:38 -
@@ -72,7 +72,6 @@ extern struct pool fusefs_fbuf_pool;
 
 /* fuse helpers */
 #define TSLEEP_TIMEOUT 5
-void update_vattr(struct mount *mp, struct vattr *v);
 
 /* files helpers. */
 int fusefs_file_open(struct fusefs_mnt *, struct fusefs_node *, enum fufh_type,

ksh.1: $() quirk note

[Martin Natano]

ksh.1 states:

Note: $(command) expressions are currently parsed by finding the 
matching
parenthesis, regardless of quoting.  This should be fixed soon.

Seems like the "soon" mentioned here is already past.

$ echo $(echo "closing: )")
closing: )
$

Ok? Am I missing something?

natano


Index: ksh.1
===
RCS file: /cvs/src/bin/ksh/ksh.1,v
retrieving revision 1.179
diff -u -p -r1.179 ksh.1
--- ksh.1   27 Apr 2016 12:46:23 -  1.179
+++ ksh.1   18 Aug 2016 07:56:10 -
@@ -1007,12 +1007,6 @@ has the same effect as
 .Ic $(cat foo) ,
 but it is carried out more efficiently because no process is started.
 .Pp
-.Sy Note :
-.Pf $( Ar command )
-expressions are currently parsed by finding the matching parenthesis,
-regardless of quoting.
-This should be fixed soon.
-.Pp
 Arithmetic substitutions are replaced by the value of the specified expression.
 For example, the command
 .Ic echo $((2+3*4))

Re: fuse cache shenanigans

[Martin Natano]

On Mon, Aug 15, 2016 at 02:11:38PM -0600, Bob Beck wrote:
> Note - NFS has similar behaviour ;)
> 
> at least within a directory.  - so this isn't tht "unusual" for non-local

NFS is much smarter than that. It verifies the results returned by
cache_lookup() with some "clever" voodoo that checks the mtime of the
directory (the vnode attributes are cached themselves, but with a
timeout), before believing any cache result.

To implement a cache correctly we would need domain knowledge of the
filesystem, which we can't have because that's the whole point of fuse.
It might be a simple "purge on create/mknod/etc." as in the case of
local filesystems or "ask $deity for advice by sacrificing dead body
parts" as in the case of nfs.


> I'm wondering if this isn't a bit premature Have you looked for other side
> effects of this removal?

I can't find any side effect beside of it being slower. As far as I can
tell the cache as used by fuse is strictly optional.


> 
> 
> On Mon, Aug 15, 2016 at 1:52 PM, Ted Unangst <t...@tedunangst.com> wrote:
> 
> > Martin Natano wrote:
> > > Watch this:
> > >
> > >   $ doas sshfs natano@localhost:/tmp /mnt
> > >   $ cat /mnt/foo
> > >   cat: /mnt/foo: No such file or directory
> > >   $ echo bar > /tmp/foo
> > >   $ cat /mnt/foo
> > >   cat: /mnt/foo: No such file or directory
> > >   $ touch /mnt/foo
> > >   $ cat /mnt/foo
> > >   bar
> > >   $
> > >
> > > There is no sense in doing caching in fusefs. In case of a non-local
> > > filesystem the tree can change behind our back without us having a
> > > chance to purge the cache.
> > >
> > > "The only winning move is not to play." Ok?
> >
> > ok
> >
> >

Re: cvs(1): default to a)bort for empty commit log

[Martin Natano]

On Mon, Aug 15, 2016 at 12:46:23PM +0200, Theo Buehler wrote:
> This is one of the quirks of cvs that I have a hard time getting used to:
> 
> $ cvs commit
> 
> Log message unchanged or not specified
> a)bort, c)ontinue, e)dit, !)reuse this message unchanged for remaining dirs
> Action: (continue)
> 
> I don't think I'll ever want to c)ontinue here because I don't want to
> commit anything without a log message.  I exit the editor without
> writing the log file precisely because there might be something wrong,
> something I need to go double check.  In that situation, I don't need
> the extra thrill of making sure that I choose a non-default option so I
> don't change anything.
> 
> Thus, I suggest to default to abort here.

The diff reads fine. Please commit, preferably including a log
message. ;)


> 
> Index: src/logmsg.c
> ===
> RCS file: /var/cvs/src/gnu/usr.bin/cvs/src/logmsg.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 logmsg.c
> --- src/logmsg.c  4 Mar 2012 04:05:15 -   1.4
> +++ src/logmsg.c  15 Aug 2016 03:25:27 -
> @@ -346,7 +346,7 @@ do_editor (dir, messagep, repository, ch
>   {
>   (void) printf ("\nLog message unchanged or not specified\n");
>   (void) printf ("a)bort, c)ontinue, e)dit, !)reuse this message 
> unchanged for remaining dirs\n");
> - (void) printf ("Action: (continue) ");
> + (void) printf ("Action: (abort) ");
>   (void) fflush (stdout);
>   line_length = get_line (, _chars_allocated, stdin);
>   if (line_length < 0)
> @@ -358,14 +358,14 @@ do_editor (dir, messagep, repository, ch
>   error (1, 0, "aborting");
>   }
>   else if (line_length == 0
> -  || *line == '\n' || *line == 'c' || *line == 'C')
> - break;
> - if (*line == 'a' || *line == 'A')
> +  || *line == '\n' || *line == 'a' || *line == 'A')
>   {
>   if (unlink_file (fname) < 0)
>   error (0, errno, "warning: cannot remove temp file %s", 
> fname);
>   error (1, 0, "aborted by user");
>   }
> + if (*line == 'c' || *line == 'C')
> + break;  
>   if (*line == 'e' || *line == 'E')
>   goto again;
>   if (*line == '!')
> 

Re: nitems not from param.h

[Martin Natano]

Ok.

fuse cache shenanigans

[Martin Natano]

Watch this:

$ doas sshfs natano@localhost:/tmp /mnt
$ cat /mnt/foo
cat: /mnt/foo: No such file or directory
$ echo bar > /tmp/foo
$ cat /mnt/foo
cat: /mnt/foo: No such file or directory
$ touch /mnt/foo
$ cat /mnt/foo
bar
$

There is no sense in doing caching in fusefs. In case of a non-local
filesystem the tree can change behind our back without us having a
chance to purge the cache.

"The only winning move is not to play." Ok?

natano


Index: miscfs/fuse/fuse_lookup.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_lookup.c,v
retrieving revision 1.12
diff -u -p -r1.12 fuse_lookup.c
--- miscfs/fuse/fuse_lookup.c   12 Aug 2016 20:18:44 -  1.12
+++ miscfs/fuse/fuse_lookup.c   15 Aug 2016 10:01:41 -
@@ -64,9 +64,6 @@ fusefs_lookup(void *v)
(cnp->cn_nameiop == DELETE || cnp->cn_nameiop == RENAME))
return (EROFS);
 
-   if ((error = cache_lookup(vdp, vpp, cnp)) >= 0)
-   return (error);
-
if (flags & ISDOTDOT) {
/* got ".." */
nid = dp->parent;
@@ -120,10 +117,8 @@ fusefs_lookup(void *v)
 * Write access to directory required to delete files.
 */
error = VOP_ACCESS(vdp, VWRITE, cred, cnp->cn_proc);
-   if (error != 0) {
-   fb_delete(fbuf);
-   return (error);
-   }
+   if (error)
+   goto out;
 
cnp->cn_flags |= SAVENAME;
}
@@ -132,10 +127,8 @@ fusefs_lookup(void *v)
/*
 * Write access to directory required to delete files.
 */
-   if ((error = VOP_ACCESS(vdp, VWRITE, cred, cnp->cn_proc)) != 0) 
{
-   fb_delete(fbuf);
-   return (error);
-   }
+   if ((error = VOP_ACCESS(vdp, VWRITE, cred, cnp->cn_proc)) != 0)
+   goto out;
 
if (nid == VTOI(vdp)->ufs_ino.i_number) {
error = EISDIR;
@@ -187,10 +180,8 @@ fusefs_lookup(void *v)
 
update_vattr(fmp->mp, >fb_vattr);
 
-   if (error) {
-   fb_delete(fbuf);
-   return (error);
-   }
+   if (error)
+   goto out;
 
if (vdp != NULL && vdp->v_type == VDIR)
VTOI(tdp)->parent = dp->ufs_ino.i_number;
@@ -204,10 +195,6 @@ fusefs_lookup(void *v)
}
 
 out:
-   if ((cnp->cn_flags & MAKEENTRY) && nameiop != CREATE &&
-   nameiop != DELETE)
-   cache_enter(vdp, *vpp, cnp);
-
fb_delete(fbuf);
return (error);
 }
Index: miscfs/fuse/fuse_vnops.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_vnops.c,v
retrieving revision 1.29
diff -u -p -r1.29 fuse_vnops.c
--- miscfs/fuse/fuse_vnops.c12 Aug 2016 20:18:44 -  1.29
+++ miscfs/fuse/fuse_vnops.c15 Aug 2016 10:01:41 -
@@ -857,7 +857,6 @@ fusefs_reclaim(void *v)
 * Remove the inode from its hash chain.
 */
ufs_ihashrem(>ufs_ino);
-   cache_purge(vp);
 
free(ip, M_FUSEFS, 0);
vp->v_data = NULL;
@@ -1386,11 +1385,9 @@ fusefs_rmdir(void *v)
goto out;
}
 
-   cache_purge(dvp);
vput(dvp);
dvp = NULL;
 
-   cache_purge(ITOV(ip));
fb_delete(fbuf);
 out:
if (dvp)

Re: a few userspace quad_t conversions

[Martin Natano]

On Sun, Aug 14, 2016 at 09:59:21PM -0700, Philip Guenther wrote:
> 
> du is an easy on, though the use of fts_number here reminded me that we 
> need to increase to a long long at some point like the other BSDs did.
> 
> hexdump is...subtle, as it both creates format strings on the fly and 
> *rewrites* them!  Turns out it was using %q for both of those, hah!  The 
> display.c change is a straight forward quad_t-->int64_t conversion, while 
> the parse.c change is to use %ll instead of %q.
> 
> systat just needs u_quad_t to be converted to uint64_t, but while we're 
> here let's convert bcopy() to memcpy() and bzero() to memset().
> 
> ok?

QK, reads fine to me. See one comment below.


> 
> PHilip
> 
> Index: usr.bin/du/du.c
> ===
> RCS file: /data/src/openbsd/src/usr.bin/du/du.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 du.c
> --- usr.bin/du/du.c   10 Oct 2015 05:32:52 -  1.31
> +++ usr.bin/du/du.c   15 Aug 2016 04:08:25 -
> @@ -50,7 +50,7 @@
>  
>  
>  int   linkchk(FTSENT *);
> -void  prtout(quad_t, char *, int);
> +void  prtout(int64_t, char *, int);
>  void  usage(void);
>  
>  int
> @@ -59,7 +59,7 @@ main(int argc, char *argv[])
>   FTS *fts;
>   FTSENT *p;
>   long blocksize;
> - quad_t totalblocks;
> + int64_t totalblocks;
>   int ftsoptions, listfiles, maxdepth;
>   int Hflag, Lflag, cflag, hflag, kflag;
>   int ch, notused, rval;
> @@ -177,7 +177,7 @@ main(int argc, char *argv[])
>* root of a traversal, display the total.
>*/
>   if (p->fts_level <= maxdepth)
> - prtout((quad_t)howmany(p->fts_number,
> + prtout(howmany(p->fts_number,
>   (unsigned long)blocksize), p->fts_path,
>   hflag);
>   break;
> @@ -207,7 +207,7 @@ main(int argc, char *argv[])
>   if (errno)
>   err(1, "fts_read");
>   if (cflag) {
> - prtout((quad_t)howmany(totalblocks, blocksize), "total", hflag);
> + prtout(howmany(totalblocks, blocksize), "total", hflag);
>   }
>   fts_close(fts);
>   exit(rval);
> @@ -301,17 +301,17 @@ linkchk(FTSENT *p)
>  }
>  
>  void
> -prtout(quad_t size, char *path, int hflag)
> +prtout(int64_t size, char *path, int hflag)
>  {
>   if (!hflag)
> - (void)printf("%lld\t%s\n", (long long)size, path);
> + (void)printf("%lld\t%s\n", size, path);
>   else {
>   char buf[FMT_SCALED_STRSIZE];
>  
>   if (fmt_scaled(size * 512, buf) == 0)
>   (void)printf("%s\t%s\n", buf, path);
>   else
> - (void)printf("%lld\t%s\n", (long long)size, path);
> + (void)printf("%lld\t%s\n", size, path);
>   }
>  }
>  
> Index: usr.bin/hexdump/display.c
> ===
> RCS file: /data/src/openbsd/src/usr.bin/hexdump/display.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 display.c
> --- usr.bin/hexdump/display.c 15 Mar 2016 04:19:13 -  1.24
> +++ usr.bin/hexdump/display.c 15 Aug 2016 04:16:35 -
> @@ -100,7 +100,7 @@ display(void)
>   for (pr = endfu->nextpr; pr; pr = pr->nextpr)
>   switch(pr->flags) {
>   case F_ADDRESS:
> - (void)printf(pr->fmt, (quad_t)eaddress);
> + (void)printf(pr->fmt, (int64_t)eaddress);
>   break;
>   case F_TEXT:
>   (void)printf("%s", pr->fmt);
> @@ -123,7 +123,7 @@ print(PR *pr, u_char *bp)
>  
>   switch(pr->flags) {
>   case F_ADDRESS:
> - (void)printf(pr->fmt, (quad_t)address);
> + (void)printf(pr->fmt, (int64_t)address);
>   break;
>   case F_BPAD:
>   (void)printf(pr->fmt, "");
> @@ -149,15 +149,15 @@ print(PR *pr, u_char *bp)
>   case F_INT:
>   switch(pr->bcnt) {
>   case 1:
> - (void)printf(pr->fmt, (quad_t)*bp);
> + (void)printf(pr->fmt, (int64_t)*bp);
>   break;
>   case 2:
>   memmove(, bp, sizeof(s2));
> - (void)printf(pr->fmt, (quad_t)s2);
> + (void)printf(pr->fmt, (int64_t)s2);
>   break;
>   case 4:
>   memmove(, bp, sizeof(s4));
> - (void)printf(pr->fmt, (quad_t)s4);
> + (void)printf(pr->fmt, (int64_t)s4);
>   break;
>   case 8:
>   memmove(, bp, sizeof(s8));
> @@ -180,15 +180,15 @@ print(PR *pr, u_char *bp)
>   case F_UINT:
>   switch(pr->bcnt) {
>

Re: csh: don't reinvent rlim_t

[Martin Natano]

On Sun, Aug 14, 2016 at 11:49:00AM -0700, Philip Guenther wrote:
> On Sun, Aug 14, 2016 at 1:26 AM, Martin Natano <nat...@natano.net> wrote:
> > On Sat, Aug 13, 2016 at 11:41:26PM -0700, Philip Guenther wrote:
> ...
> >> @@ -1124,14 +1122,14 @@ getval(struct limits *lp, Char **v)
> >>   cp++;
> >>  if (*cp == 0) {
> >>   if (*v == 0)
> >> - return ((RLIM_TYPE) ((f + 0.5) * lp->limdiv));
> >> + return ((rlim_t) ((f + 0.5) * lp->limdiv));
> >
> > Shouldn't 'return (f + 0.5) * lp->limdiv;' be enough here, because
> > rlim_t is the return type?
> 
> This is perhaps an inconsistency in my preferences, but I mildly
> prefer the cast here because it's a potentially lossy float->integer
> conversion.

This 'inconsistency' is indeed what made me comment here. I guess in the
float->int case the cast is ok though. Feel free to commit including the
cast.


> 
> Philip

fuse: fh functions must die

[Martin Natano]

After staring a couple of days at fusefs and libfuse code I came to the
conclusion, that the fuse_vptofh() and fuse_fhtovp() functions must die.
I implemented those functions under the assumption, that fuse_vget() has
reasonable semantics, while this is not the case. fusefs_vget() only
functions correctly, if the file in questions has recently been accessed
and is still in the vnode cache of the userspace daemon associated with
the mount point.

As a matter of fact the fuse api doesn't feature a reasonable way to map
a inode number to a handle at all (see struct fuse_operations).

I don't this this will be missed, as the only thing in base using it is
nfs and the getfh/fhstat/fhopen functions which are not used in base
apart from nfs support code.

Ok?


Index: miscfs/fuse/fuse_vfsops.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_vfsops.c,v
retrieving revision 1.25
diff -u -p -r1.25 fuse_vfsops.c
--- miscfs/fuse/fuse_vfsops.c   13 Aug 2016 11:42:46 -  1.25
+++ miscfs/fuse/fuse_vfsops.c   14 Aug 2016 14:34:54 -
@@ -300,28 +300,13 @@ retry:
 int
 fusefs_fhtovp(struct mount *mp, struct fid *fhp, struct vnode **vpp)
 {
-   struct ufid *ufhp;
-
-   ufhp = (struct ufid *)fhp;
-   if (ufhp->ufid_len != sizeof(struct ufid) ||
-   ufhp->ufid_ino < FUSE_ROOTINO)
-   return (ESTALE);
-
-   return (VFS_VGET(mp, ufhp->ufid_ino, vpp));
+   return (EINVAL);
 }
 
 int
 fusefs_vptofh(struct vnode *vp, struct fid *fhp)
 {
-   struct fusefs_node *ip;
-   struct ufid *ufhp;
-
-   ip = VTOI(vp);
-   ufhp = (struct ufid *)fhp;
-   ufhp->ufid_len = sizeof(struct ufid);
-   ufhp->ufid_ino = ip->ufs_ino.i_number;
-
-   return (0);
+   return (EINVAL);
 }
 
 int

Re: eliminate uses of strtoq/strtouq

[Martin Natano]

> Index: usr.sbin/mtree/spec.c
> ===
> RCS file: /data/src/openbsd/src/usr.sbin/mtree/spec.c,v
> retrieving revision 1.26
> diff -u -p -r1.26 spec.c
> --- usr.sbin/mtree/spec.c 8 Jul 2012 21:19:42 -   1.26
> +++ usr.sbin/mtree/spec.c 14 Aug 2016 07:38:44 -
> @@ -241,7 +241,7 @@ set(char *t, NODE *ip)
>   error("%s", strerror(errno));
>   break;
>   case F_SIZE:
> - ip->st_size = strtouq(val, , 10);
> + ip->st_size = strtoull(val, , 10);
>   if (*ep)
>   error("invalid size %s", val);
>   break;

st_size is an off_t, so shouldn't that rather be strtoll? Also, mtree
uses %q in a printf format in two places. Otherwise the diff makes sense
to me.

natano

Re: reduce qabs() and qdiv() to aliases and deprecate them in manual

[Martin Natano]

On Sun, Aug 14, 2016 at 12:25:18AM -0700, Philip Guenther wrote:
> 
> strtoq() and strtouq() are already provided as just aliases of strtoll() 
> and strotull(); let's do the same with qabs() and qdiv() instead of having 
> them as fully separate functions.
> 
> Similarly, they don't need separate manpages, so roll them into the same 
> manpages as llabs(3) and lldiv(3), call the deprecated, and kill the Xr's 
> to the old pages.  You'll need to manually remove 
> /usr/share/man/q{abs,div}.3 and run makewhatis to get it to show the new 
> versions, I believe.
> 
> ok?

OK.


> 
> Philip
> 
> 
> Index: hidden/stdlib.h
> ===
> RCS file: /data/src/openbsd/src/lib/libc/hidden/stdlib.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 stdlib.h
> --- hidden/stdlib.h   13 Mar 2016 18:34:21 -  1.7
> +++ hidden/stdlib.h   14 Aug 2016 06:59:39 -
> @@ -119,8 +119,8 @@ PROTO_DEPRECATED(nrand48);
>  PROTO_DEPRECATED(posix_openpt);
>  PROTO_DEPRECATED(ptsname);
>  PROTO_NORMAL(putenv);
> -PROTO_DEPRECATED(qabs);
> -PROTO_DEPRECATED(qdiv);
> +/*PROTO_DEPRECATED(qabs);alias of llabs */
> +/*PROTO_DEPRECATED(qdiv);alias of lldiv */
>  PROTO_NORMAL(qsort);
>  PROTO_DEPRECATED(radixsort);
>  PROTO_STD_DEPRECATED(rand);
> Index: stdlib/Makefile.inc
> ===
> RCS file: /data/src/openbsd/src/lib/libc/stdlib/Makefile.inc,v
> retrieving revision 1.60
> diff -u -p -r1.60 Makefile.inc
> --- stdlib/Makefile.inc   2 May 2016 12:59:24 -   1.60
> +++ stdlib/Makefile.inc   14 Aug 2016 07:13:40 -
> @@ -11,8 +11,8 @@ SRCS+=  a64l.c abort.c atexit.c atoi.c at
>   realpath.c remque.c setenv.c strtoimax.c \
>   strtol.c strtoll.c strtonum.c strtoul.c strtoull.c strtoumax.c \
>   system.c tfind.c tsearch.c _rand48.c drand48.c erand48.c jrand48.c \
> - lcong48.c lrand48.c mrand48.c nrand48.c seed48.c srand48.c qabs.c \
> - qdiv.c _Exit.c icdb.c
> + lcong48.c lrand48.c mrand48.c nrand48.c seed48.c srand48.c \
> + _Exit.c icdb.c
>  
>  .if (${MACHINE_CPU} == "i386")
>  SRCS+=   abs.S div.S labs.S ldiv.S
> @@ -27,5 +27,5 @@ MAN+=   a64l.3 abort.3 abs.3 alloca.3 atex
>   bsearch.3 div.3 ecvt.3 exit.3 getenv.3 getopt.3 getopt_long.3 \
>   getsubopt.3 hcreate.3 imaxabs.3 imaxdiv.3 insque.3 labs.3 ldiv.3 \
>   lldiv.3 lsearch.3 malloc.3 posix_memalign.3 posix_openpt.3 ptsname.3 \
> - qabs.3 qdiv.3 qsort.3 radixsort.3 rand48.3 rand.3 random.3 realpath.3 \
> + qsort.3 radixsort.3 rand48.3 rand.3 random.3 realpath.3 \
>   strtod.3 strtonum.3 strtol.3 strtoul.3 system.3 tsearch.3
> Index: stdlib/div.3
> ===
> RCS file: /data/src/openbsd/src/lib/libc/stdlib/div.3,v
> retrieving revision 1.11
> diff -u -p -r1.11 div.3
> --- stdlib/div.3  5 Jun 2013 03:39:23 -   1.11
> +++ stdlib/div.3  14 Aug 2016 07:12:29 -
> @@ -55,8 +55,7 @@ and
>  .Sh SEE ALSO
>  .Xr imaxdiv 3 ,
>  .Xr ldiv 3 ,
> -.Xr lldiv 3 ,
> -.Xr qdiv 3
> +.Xr lldiv 3
>  .Sh STANDARDS
>  The
>  .Fn div
> Index: stdlib/imaxdiv.3
> ===
> RCS file: /data/src/openbsd/src/lib/libc/stdlib/imaxdiv.3,v
> retrieving revision 1.6
> diff -u -p -r1.6 imaxdiv.3
> --- stdlib/imaxdiv.3  30 Nov 2014 21:21:59 -  1.6
> +++ stdlib/imaxdiv.3  14 Aug 2016 07:12:34 -
> @@ -57,8 +57,7 @@ and
>  .Sh SEE ALSO
>  .Xr div 3 ,
>  .Xr ldiv 3 ,
> -.Xr lldiv 3 ,
> -.Xr qdiv 3
> +.Xr lldiv 3
>  .Sh STANDARDS
>  The
>  .Fn imaxdiv
> Index: stdlib/labs.3
> ===
> RCS file: /data/src/openbsd/src/lib/libc/stdlib/labs.3,v
> retrieving revision 1.13
> diff -u -p -r1.13 labs.3
> --- stdlib/labs.3 30 Nov 2014 21:21:59 -  1.13
> +++ stdlib/labs.3 14 Aug 2016 07:11:36 -
> @@ -44,6 +44,8 @@
>  .Fn labs "long i"
>  .Ft long long
>  .Fn llabs "long long j"
> +.Ft quad_t
> +.Fn qabs "quad_t j"
>  .Sh DESCRIPTION
>  The
>  .Fn labs
> @@ -53,6 +55,11 @@ The
>  .Fn llabs
>  function returns the absolute value of the long long integer
>  .Fa j .
> +The
> +.Fn qabs
> +function is a deprecated equivalent of
> +.Fn llabs
> +and is provided for backwards compatibility with legacy programs.
>  .Sh SEE ALSO
>  .Xr abs 3 ,
>  .Xr cabs 3 ,
> Index: stdlib/ldiv.3
> ===
> RCS file: /data/src/openbsd/src/lib/libc/stdlib/ldiv.3,v
> retrieving revision 1.12
> diff -u -p -r1.12 ldiv.3
> --- stdlib/ldiv.3 17 Jul 2013 05:42:11 -  1.12
> +++ stdlib/ldiv.3 14 Aug 2016 07:09:58 -
> @@ -57,8 +57,7 @@ and
>  .Sh SEE ALSO
>  .Xr div 3 ,
>  .Xr imaxdiv 3 ,
> -.Xr lldiv 3 ,
> -.Xr qdiv 3
> +.Xr lldiv 3
>  .Sh STANDARDS
>  The
>  .Fn ldiv
> Index: stdlib/llabs.c
> 

Re: csh: don't reinvent rlim_t

[Martin Natano]

On Sat, Aug 13, 2016 at 11:41:26PM -0700, Philip Guenther wrote:
> 
> This seems unnecessary:
>   typedef quad_t RLIM_TYPE;
> 
> when we have rlim_t.  Yes, rlim_t is unsigned while RLIM_TYPE is signed, 
> but it's not doing anything with this that needs a signed type.
> 
> While ktracing csh to verify the setrlimit/getrlimit calls, I noticed that 
> csh calls getrlimit 513(!) times on startup.  Thus the diff to misc.c to 
> pull the sysconf(_SC_OPEN_MAX) out of the loop conditional...
> 
> ok?

Reads fine to me. See one comment below.


> 
> Philip Guenther
> 
> Index: func.c
> ===
> RCS file: /data/src/openbsd/src/bin/csh/func.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 func.c
> --- func.c26 Dec 2015 13:48:38 -  1.32
> +++ func.c14 Aug 2016 06:26:28 -
> @@ -1036,8 +1036,6 @@ doumask(Char **v, struct command *t)
>  (void) umask(i);
>  }
>  
> -typedef quad_t RLIM_TYPE;
> -
>  static struct limits {
>  int limconst;
>  char   *limname;
> @@ -1060,10 +1058,10 @@ static struct limits {
>  };
>  
>  static struct limits *findlim(Char *);
> -static RLIM_TYPE getval(struct limits *, Char **);
> +static rlim_t getval(struct limits *, Char **);
>  static void limtail(Char *, char *);
>  static void plim(struct limits *, Char);
> -static int setlim(struct limits *, Char, RLIM_TYPE);
> +static int setlim(struct limits *, Char, rlim_t);
>  
>  static struct limits *
>  findlim(Char *cp)
> @@ -1089,7 +1087,7 @@ void
>  dolimit(Char **v, struct command *t)
>  {
>  struct limits *lp;
> -RLIM_TYPE limit;
> +rlim_t limit;
>  charhard = 0;
>  
>  v++;
> @@ -1112,7 +1110,7 @@ dolimit(Char **v, struct command *t)
>   stderror(ERR_SILENT);
>  }
>  
> -static  RLIM_TYPE
> +static  rlim_t
>  getval(struct limits *lp, Char **v)
>  {
>  float f;
> @@ -1124,14 +1122,14 @@ getval(struct limits *lp, Char **v)
>   cp++;
>  if (*cp == 0) {
>   if (*v == 0)
> - return ((RLIM_TYPE) ((f + 0.5) * lp->limdiv));
> + return ((rlim_t) ((f + 0.5) * lp->limdiv));

Shouldn't 'return (f + 0.5) * lp->limdiv;' be enough here, because
rlim_t is the return type?


>   cp = *v;
>  }
>  switch (*cp) {
>  case ':':
>   if (lp->limconst != RLIMIT_CPU)
>   goto badscal;
> - return ((RLIM_TYPE) (f * 60.0 + atof(short2str(cp + 1;
> + return ((rlim_t) (f * 60.0 + atof(short2str(cp + 1;

ditto

>  case 'h':
>   if (lp->limconst != RLIMIT_CPU)
>   goto badscal;
> @@ -1177,7 +1175,7 @@ badscal:
>  if (f > (float) RLIM_INFINITY)
>   return RLIM_INFINITY;
>  else
> - return ((RLIM_TYPE) f);
> + return ((rlim_t) f);

ditto


>  }
>  
>  static void
> @@ -1196,7 +1194,7 @@ static void
>  plim(struct limits *lp, Char hard)
>  {
>  struct rlimit rlim;
> -RLIM_TYPE limit;
> +rlim_t limit;
>  
>  (void) fprintf(cshout, "%s \t", lp->limname);
>  
> @@ -1208,8 +1206,8 @@ plim(struct limits *lp, Char hard)
>  else if (lp->limconst == RLIMIT_CPU)
>   psecs((long) limit);
>  else
> - (void) fprintf(cshout, "%ld %s", (long) (limit / lp->limdiv),
> -lp->limscale);
> + (void) fprintf(cshout, "%llu %s",
> + (unsigned long long) (limit / lp->limdiv), lp->limscale);
>  (void) fputc('\n', cshout);
>  }
>  
> @@ -1228,7 +1226,7 @@ dounlimit(Char **v, struct command *t)
>  }
>  if (*v == 0) {
>   for (lp = limits; lp->limconst >= 0; lp++)
> - if (setlim(lp, hard, (RLIM_TYPE) RLIM_INFINITY) < 0)
> + if (setlim(lp, hard, RLIM_INFINITY) < 0)
>   lerr++;
>   if (lerr)
>   stderror(ERR_SILENT);
> @@ -1236,13 +1234,13 @@ dounlimit(Char **v, struct command *t)
>  }
>  while (*v) {
>   lp = findlim(*v++);
> - if (setlim(lp, hard, (RLIM_TYPE) RLIM_INFINITY) < 0)
> + if (setlim(lp, hard, RLIM_INFINITY) < 0)
>   stderror(ERR_SILENT);
>  }
>  }
>  
>  static int
> -setlim(struct limits *lp, Char hard, RLIM_TYPE limit)
> +setlim(struct limits *lp, Char hard, rlim_t limit)
>  {
>  struct rlimit rlim;
>  
> Index: misc.c
> ===
> RCS file: /data/src/openbsd/src/bin/csh/misc.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 misc.c
> --- misc.c26 Dec 2015 13:48:38 -  1.18
> +++ misc.c14 Aug 2016 06:04:53 -
> @@ -170,8 +170,9 @@ void
>  closem(void)
>  {
>  int f;
> +int max = sysconf(_SC_OPEN_MAX);
>  
> -for (f = 0; f < sysconf(_SC_OPEN_MAX); f++)
> +for (f = 0; f < max; f++)
>   if (f != SHIN && f != SHOUT && f != SHERR && f != OLDSTD &&
>   f != FSHTTY)
>   (void) close(f);
> 

kill FUSE_ROOT_ID

[Martin Natano]

Kill FUSE_ROOT_ID and use FUSE_ROOTINO instead for the only place where
it was used. Also, remove one (ino_t) cast from FUSE_ROOTINO, as it is
already included in the #define. Ok?

natano


Index: miscfs/fuse/fuse_vfsops.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_vfsops.c,v
retrieving revision 1.24
diff -u -p -r1.24 fuse_vfsops.c
--- miscfs/fuse/fuse_vfsops.c   12 Aug 2016 20:18:44 -  1.24
+++ miscfs/fuse/fuse_vfsops.c   13 Aug 2016 10:53:23 -
@@ -172,7 +172,7 @@ fusefs_root(struct mount *mp, struct vno
struct vnode *nvp;
int error;
 
-   if ((error = VFS_VGET(mp, (ino_t)FUSE_ROOTINO, )) != 0)
+   if ((error = VFS_VGET(mp, FUSE_ROOTINO, )) != 0)
return (error);
 
nvp->v_type = VDIR;
@@ -200,7 +200,7 @@ fusefs_statfs(struct mount *mp, struct s
copy_statfs_info(sbp, mp);
 
if (fmp->sess_init) {
-   fbuf = fb_setup(0, FUSE_ROOT_ID, FBT_STATFS, p);
+   fbuf = fb_setup(0, FUSE_ROOTINO, FBT_STATFS, p);
 
error = fb_queue(fmp->dev, fbuf);
 
Index: sys/fusebuf.h
===
RCS file: /cvs/src/sys/sys/fusebuf.h,v
retrieving revision 1.9
diff -u -p -r1.9 fusebuf.h
--- sys/fusebuf.h   16 Jan 2014 09:31:44 -  1.9
+++ sys/fusebuf.h   13 Aug 2016 10:53:23 -
@@ -133,9 +133,6 @@ struct fusebuf {
 
 #ifdef _KERNEL
 
-/* The node ID of the root inode */
-#define FUSE_ROOT_ID   1
-
 /* fusebuf prototypes */
 struct fusebuf *fb_setup(size_t, ino_t, int, struct proc *);
 intfb_queue(dev_t, struct fusebuf *);

fuse: dedup vnode type

[Martin Natano]

Fuse stores the vnode type in two places: vtype in struct fusefs_node
and v_type in struct vnode. Given the fact, that fusefs_node structs are
never allocated without an associated vnode and those two fields are
always in sync, one of those locations is superfluous.

Ok?

natano


Index: miscfs/fuse/fuse_lookup.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_lookup.c,v
retrieving revision 1.11
diff -u -p -r1.11 fuse_lookup.c
--- miscfs/fuse/fuse_lookup.c   19 Mar 2016 12:04:15 -  1.11
+++ miscfs/fuse/fuse_lookup.c   11 Aug 2016 10:26:51 -
@@ -147,7 +147,6 @@ fusefs_lookup(void *v)
goto out;
 
tdp->v_type = IFTOVT(fbuf->fb_vattr.va_mode);
-   VTOI(tdp)->vtype = tdp->v_type;
*vpp = tdp;
cnp->cn_flags |= SAVENAME;
 
@@ -183,10 +182,8 @@ fusefs_lookup(void *v)
} else {
error = VFS_VGET(fmp->mp, nid, );
 
-   if (!error) {
+   if (!error)
tdp->v_type = IFTOVT(fbuf->fb_vattr.va_mode);
-   VTOI(tdp)->vtype = tdp->v_type;
-   }
 
update_vattr(fmp->mp, >fb_vattr);
 
Index: miscfs/fuse/fuse_vfsops.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_vfsops.c,v
retrieving revision 1.23
diff -u -p -r1.23 fuse_vfsops.c
--- miscfs/fuse/fuse_vfsops.c   19 Jun 2016 11:54:33 -  1.23
+++ miscfs/fuse/fuse_vfsops.c   11 Aug 2016 10:26:51 -
@@ -170,15 +170,12 @@ int
 fusefs_root(struct mount *mp, struct vnode **vpp)
 {
struct vnode *nvp;
-   struct fusefs_node *ip;
int error;
 
if ((error = VFS_VGET(mp, (ino_t)FUSE_ROOTINO, )) != 0)
return (error);
 
-   ip = VTOI(nvp);
nvp->v_type = VDIR;
-   ip->vtype = VDIR;
 
*vpp = nvp;
return (0);
Index: miscfs/fuse/fuse_vnops.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_vnops.c,v
retrieving revision 1.28
diff -u -p -r1.28 fuse_vnops.c
--- miscfs/fuse/fuse_vnops.c19 Jun 2016 11:54:33 -  1.28
+++ miscfs/fuse/fuse_vnops.c11 Aug 2016 10:26:51 -
@@ -235,7 +235,7 @@ fusefs_open(void *v)
return (ENXIO);
 
isdir = 0;
-   if (ip->vtype == VDIR)
+   if (ap->a_vp->v_type == VDIR)
isdir = 1;
else {
if ((ap->a_mode & FREAD) && (ap->a_mode & FWRITE)) {
@@ -274,7 +274,7 @@ fusefs_close(void *v)
if (!fmp->sess_init)
return (0);
 
-   if (ip->vtype == VDIR) {
+   if (ap->a_vp->v_type == VDIR) {
isdir = 1;
 
if (ip->fufh[fufh_type].fh_type != FUFH_INVALID)
@@ -665,7 +665,6 @@ fusefs_symlink(void *v)
}
 
tdp->v_type = VLNK;
-   VTOI(tdp)->vtype = tdp->v_type;
VTOI(tdp)->parent = dp->ufs_ino.i_number;
VN_KNOTE(ap->a_dvp, NOTE_WRITE);
 
@@ -762,7 +761,7 @@ fusefs_inactive(void *v)
fufh = &(ip->fufh[type]);
if (fufh->fh_type != FUFH_INVALID)
fusefs_file_close(fmp, ip, fufh->fh_type, type,
-   (ip->vtype == VDIR), ap->a_p);
+   (vp->v_type == VDIR), ap->a_p);
}
 
error = VOP_GETATTR(vp, , cred, p);
@@ -835,7 +834,7 @@ fusefs_reclaim(void *v)
if (fufh->fh_type != FUFH_INVALID) {
printf("fusefs: vnode being reclaimed is valid\n");
fusefs_file_close(fmp, ip, fufh->fh_type, type,
-   (ip->vtype == VDIR), ap->a_p);
+   (vp->v_type == VDIR), ap->a_p);
}
}
/*
@@ -932,8 +931,6 @@ fusefs_create(void *v)
}
 
tdp->v_type = IFTOVT(fbuf->fb_io_mode);
-   VTOI(tdp)->vtype = tdp->v_type;
-
if (dvp != NULL && dvp->v_type == VDIR)
VTOI(tdp)->parent = ip->ufs_ino.i_number;
 
@@ -998,8 +995,6 @@ fusefs_mknod(void *v)
}
 
tdp->v_type = IFTOVT(fbuf->fb_io_mode);
-   VTOI(tdp)->vtype = tdp->v_type;
-
if (dvp != NULL && dvp->v_type == VDIR)
VTOI(tdp)->parent = ip->ufs_ino.i_number;
 
@@ -1211,7 +1206,7 @@ abortit:
 * "ls" or "pwd" with the "." directory entry missing, and "cd .."
 * doesn't work if the ".." entry is missing.
 */
-   if (ip->vtype == VDIR) {
+   if (fvp->v_type == VDIR) {
/*
 * Avoid ".", "..", and aliases of "." for obvious reasons.
 */
@@ -1325,8 +1320,6 @@ fusefs_mkdir(void *v)
}
 
tdp->v_type = IFTOVT(fbuf->fb_io_mode);
-   VTOI(tdp)->vtype = tdp->v_type;
-
if (dvp != NULL && dvp->v_type == VDIR)
VTOI(tdp)->parent = ip->ufs_ino.i_number;
 
Index: miscfs/fuse/fusefs_node.h

Re: fuse: dedup vnode type

[Martin Natano]

> Index: miscfs/fuse/fusefs_node.h
> ===
> RCS file: /cvs/src/sys/miscfs/fuse/fusefs_node.h,v
> retrieving revision 1.2
> diff -u -p -r1.2 fusefs_node.h
> --- miscfs/fuse/fusefs_node.h 1 Feb 2014 09:30:38 -   1.2
> +++ miscfs/fuse/fusefs_node.h 11 Aug 2016 10:26:51 -
> @@ -46,8 +46,6 @@ struct fusefs_node {
>  
>   /** meta **/
>   off_t filesize;
> - uint64_t  nlookup;
> - enum vtypevtype;
>  };

While there I also removed the unused 'nlookup' field,

Re: fuse needs ufs ihash

[Martin Natano]

On Thu, Aug 11, 2016 at 10:48:59AM -0400, Ted Unangst wrote:
> Martin Natano wrote:
> > I'm already working on a diff to decouple fuse from ufs ihash. In the
> > meantime: Make sure the necessary code is compiled in when fuse is
> > enabled in the config.
> 
> Are people building kernels with FFS? This is like the old INET option, which
> I deleted because it was insane to disable it.

I don't think a kernel without ffs would be particulary useful. If you
want to remove the FFS option and noone comes up with a good reason to
keep it consider having my ok. Although as long as the option exists I
think it would be nice to have it work correctly.

Some background information why I want to untangle fuse from ufs ihash:
I believe the usage of ufs_ihash in fuse produces some ugly code in
fuse. e.g. (from fusefs_node.h):

...
#include 
...
#ifdef ITOV
# undef ITOV
#endif
#define ITOV(ip) ((ip)->ufs_ino.i_vnode)

#ifdef VTOI
# undef VTOI
#endif
#define VTOI(vp) ((struct fusefs_node *)(vp)->v_data)
...


and (from fuse_vfsops.c):

...
ip->ufs_ino.i_ump = (struct ufsmount *)fmp;
...

and (everywhere):

...
fmp = (struct fusefs_mnt *)ip->ufs_ino.i_ump;
...


Furthermove, struct fusefs_node contains a struct inode, which contains
much more fields than are required for just the ufs ihash stuff which is
actually used by fuse, resulting in a bloated fuse node structure with
most of the stuff in it unused.

Also, the dichotomy between ino_t and ufsino_t in fuse would not be
necessary when no ufs code is involved, eliminating the need to have
overflow checks in place, that should be there, but are not.

So, my idea to untangle fuse from ufs has nothing to do with trying to
build a kernel without ffs. ;) IMHO, kill the option.

natano

fuse needs ufs ihash

[Martin Natano]

I'm already working on a diff to decouple fuse from ufs ihash. In the
meantime: Make sure the necessary code is compiled in when fuse is
enabled in the config.

natano


Index: conf/files
===
RCS file: /cvs/src/sys/conf/files,v
retrieving revision 1.622
diff -u -p -r1.622 files
--- conf/files  5 Aug 2016 19:00:25 -   1.622
+++ conf/files  11 Aug 2016 08:31:16 -
@@ -896,7 +896,7 @@ file ufs/mfs/mfs_vfsops.c   mfs
 file ufs/mfs/mfs_vnops.c   mfs
 file ufs/ufs/ufs_bmap.cffs | mfs | ext2fs
 file ufs/ufs/ufs_dirhash.c ufs_dirhash & (ffs | mfs)
-file ufs/ufs/ufs_ihash.c   ffs | mfs | ext2fs
+file ufs/ufs/ufs_ihash.c   ffs | mfs | ext2fs | fuse
 file ufs/ufs/ufs_inode.c   ffs | mfs | ext2fs
 file ufs/ufs/ufs_lookup.c  ffs | mfs | ext2fs
 file ufs/ufs/ufs_quota.c   quota & ( ffs | mfs | ext2fs )

stale prototypes in ntfs_ihash.h

[Martin Natano]

Seems like ntfs_ihash.h has grown some mold; those functions don't
exist. Ok?

natano


Index: ntfs/ntfs_ihash.h
===
RCS file: /cvs/src/sys/ntfs/ntfs_ihash.h,v
retrieving revision 1.5
diff -u -p -r1.5 ntfs_ihash.h
--- ntfs/ntfs_ihash.h   30 May 2013 20:11:06 -  1.5
+++ ntfs/ntfs_ihash.h   10 Aug 2016 14:53:06 -
@@ -31,9 +31,6 @@
 
 extern struct rwlock ntfs_hashlock;
 void ntfs_nthashinit(void);
-void ntfs_nthashreinit(void);
-void ntfs_nthashdone(void);
 struct ntnode   *ntfs_nthashlookup(dev_t, ntfsino_t);
-struct ntnode   *ntfs_nthashget(dev_t, ntfsino_t);
 void ntfs_nthashins(struct ntnode *);
 void ntfs_nthashrem(struct ntnode *);

Re: ksh, ctrl-r followed by arrow key leaves "[D" or "[C" artifacts

[Martin Natano]

On Mon, Aug 08, 2016 at 03:33:23AM +0200, Ingo Schwarze wrote:
> Hi Dave,
> 
> redirecting from misc@ to tech@ because i'm appending a patch
> at the very end, lightly tested.
> 
> This has indeed been annoying me for years, but it never occurred
> to me that i might be able to figure out what's going on.
> Thanks for providing your analysis, i think it's spot on.
> 
> So the solution is to not swallow up that escape character, right?

Thats not always correct. With your patch ksh now eats the next key you
type when you exit the search prompt with the escape key. The only way
to know whether the esacpe is part of a longer sequence or was typed by
the user is timing. I've sent a patch some time ago that checks if more
bytes are available from the input descriptor to decide whether the esc
was a single one or part of a sequence:
https://marc.info/?l=openbsd-tech=141240304628749=2

Below an updated version of that patch.

natano


Index: edit.c
===
RCS file: /cvs/src/bin/ksh/edit.c,v
retrieving revision 1.53
diff -u -p -r1.53 edit.c
--- edit.c  17 Mar 2016 23:33:23 -  1.53
+++ edit.c  8 Aug 2016 06:19:34 -
@@ -10,6 +10,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -149,6 +150,16 @@ x_puts(const char *s)
 {
while (*s != 0)
shf_putc(*s++, shl_out);
+}
+
+int
+x_avail(void)
+{
+   struct pollfd pfd[1];
+
+   pfd[0].fd = STDIN_FILENO;
+   pfd[0].events = POLLIN;
+   return poll(pfd, 1, 0) == 1;
 }
 
 bool
Index: edit.h
===
RCS file: /cvs/src/bin/ksh/edit.h,v
retrieving revision 1.11
diff -u -p -r1.11 edit.h
--- edit.h  26 Jan 2016 17:39:31 -  1.11
+++ edit.h  8 Aug 2016 06:19:34 -
@@ -39,6 +39,7 @@ int   x_getc(void);
 void   x_flush(void);
 void   x_putc(int);
 void   x_puts(const char *);
+intx_avail(void);
 bool   x_mode(bool);
 intpromptlen(const char *, const char **);
 intx_do_comment(char *, int, int *);
Index: emacs.c
===
RCS file: /cvs/src/bin/ksh/emacs.c,v
retrieving revision 1.65
diff -u -p -r1.65 emacs.c
--- emacs.c 26 Jan 2016 17:39:31 -  1.65
+++ emacs.c 8 Aug 2016 06:19:35 -
@@ -893,9 +893,12 @@ x_search_hist(int c)
if ((c = x_e_getc()) < 0)
return KSTD;
f = kb_find_hist_func(c);
-   if (c == CTRL('['))
+   if (c == CTRL('[')) {
+   /* might be part of an escape sequence */
+   if (x_avail())
+   x_e_ungetc(c);
break;
-   else if (f == x_search_hist)
+   } else if (f == x_search_hist)
offset = x_search(pat, 0, offset);
else if (f == x_del_back) {
if (p == pat) {


> 
> Yours,
>   Ingo
> 
> 
> Dave Cohen wrote on Sun, Aug 07, 2016 at 04:52:50PM -0700:
> 
> > I'll try to describe an annoyance with my ksh setup.  Web and man
> > page searching has not provided a solution.  I'm relatively new to
> > both ksh and openbsd. I'm on version 5.9 release.
> > 
> > Problem happens when I navigate command history with ctrl-r, then
> > use left or right arrow.  Hitting left arrow writes "[D", right
> > inserts "[C".  I'm hitting the arrow keys so I can edit my prior
> > command.  It's a habit I'm used to that works in bash.
> > 
> > For example to reproduce, let's say I ran "ls -l" but I wanted
> > to run "ls -la"...
> > 
> > run the first command, "ls -l".
> > 
> > type "ctrl-r ls".  This works as expected, and my cursor is now
> > in the middle of "ls -l".
> > 
> > type right arrow.  This is where the problem is.  The command I'm
> > editing becomes "ls[C -l".
> > 
> > From this point, arrow keys work as expected.  I can use left or
> > right to navigate and edit the command.
> > 
> > If, instead of arrows, I use ctrl-b or ctrl-f, these work fine.
> > No artifacts like "[C" or "[D".
> > 
> > If I use bash instead of ksh, this problem does not occur.
> > 
> [...]
> > I understand from `man ksh` that these key bindings are defaults:
> >bind '^[[C'=forward-char
> >bind '^[[D'=backward-char
> > 
> > My assumption is that when in ctrl-r mode, the '^[' is interpreted
> > as part of the ctrl-r search (which doesn't match), then the '[C'
> > or '[D' is interpreted as the next key (which is inserted).  Can
> > this behavior be changed?
> 
> 
> Index: emacs.c
> ===
> RCS file: /cvs/src/bin/ksh/emacs.c,v
> retrieving revision 1.65
> diff -u -p -r1.65 emacs.c
> --- emacs.c   26 Jan 2016 17:39:31 -  1.65
> +++ emacs.c   8 Aug 2016 01:25:13 -
> @@ -893,9 +893,10 @@ x_search_hist(int c)
>   if ((c = x_e_getc()) < 0)
>

move ufs_vinit() to ffs

[Martin Natano]

The ufs_vinit() function should really be called ffs_vinit(). The only
place it is called from is ffs_vget(). And again, the FIFOOPS macro can
be killed.

Ok?

natano


Index: ufs/ffs/ffs_extern.h
===
RCS file: /cvs/src/sys/ufs/ffs/ffs_extern.h,v
retrieving revision 1.42
diff -u -p -r1.42 ffs_extern.h
--- ufs/ffs/ffs_extern.h23 May 2016 09:31:28 -  1.42
+++ ufs/ffs/ffs_extern.h7 Aug 2016 21:25:39 -
@@ -132,6 +132,7 @@ int  ffs_isfreeblock(struct fs *, u_char
 int  ffs_isblock(struct fs *, u_char *, daddr_t);
 void ffs_clrblock(struct fs *, u_char *, daddr_t);
 void ffs_setblock(struct fs *, u_char *, daddr_t);
+int  ffs_vinit(struct mount *, struct vnode **);
 
 /* ffs_vfsops.c */
 int ffs_mountroot(void);
@@ -187,12 +188,6 @@ void  softdep_setup_allocindir_page(stru
 void  softdep_fsync_mountdev(struct vnode *, int);
 int   softdep_sync_metadata(struct vop_fsync_args *);
 int   softdep_fsync(struct vnode *);
-
-#ifdef FIFO
-#define FFS_FIFOOPS _fifovops
-#else
-#define FFS_FIFOOPS NULL
-#endif
 
 extern struct pool ffs_ino_pool;   /* memory pool for inodes */
 extern struct pool ffs_dinode1_pool;   /* memory pool for UFS1 dinodes */
Index: ufs/ffs/ffs_subr.c
===
RCS file: /cvs/src/sys/ufs/ffs/ffs_subr.c,v
retrieving revision 1.30
diff -u -p -r1.30 ffs_subr.c
--- ufs/ffs/ffs_subr.c  28 Nov 2015 21:52:02 -  1.30
+++ ufs/ffs/ffs_subr.c  7 Aug 2016 21:25:39 -
@@ -244,3 +244,68 @@ ffs_isfreeblock(struct fs *fs, u_char *c
return ((cp[h >> 3] & (0x01 << (h & 0x7))) == 0);
}
 }
+
+/*
+ * Initialize the vnode associated with a new inode, handle aliased
+ * vnodes.
+ */
+int
+ffs_vinit(struct mount *mntp, struct vnode **vpp)
+{
+   struct inode *ip;
+   struct vnode *vp, *nvp;
+   struct timeval mtv;
+
+   vp = *vpp;
+   ip = VTOI(vp);
+   switch(vp->v_type = IFTOVT(DIP(ip, mode))) {
+   case VCHR:
+   case VBLK:
+   vp->v_op = _specvops;
+   if ((nvp = checkalias(vp, DIP(ip, rdev), mntp)) != NULL) {
+   /*
+* Discard unneeded vnode, but save its inode.
+* Note that the lock is carried over in the inode
+* to the replacement vnode.
+*/
+   nvp->v_data = vp->v_data;
+   vp->v_data = NULL;
+   vp->v_op = _vops;
+#ifdef VFSLCKDEBUG
+   vp->v_flag &= ~VLOCKSWORK;
+#endif
+   vrele(vp);
+   vgone(vp);
+   /*
+* Reinitialize aliased inode.
+*/
+   vp = nvp;
+   ip->i_vnode = vp;
+   }
+   break;
+   case VFIFO:
+#ifdef FIFO
+   vp->v_op = _fifovops;
+   break;
+#else
+   return (EOPNOTSUPP);
+#endif
+   case VNON:
+   case VBAD:
+   case VSOCK:
+   case VLNK:
+   case VDIR:
+   case VREG:
+   break;
+   }
+   if (ip->i_number == ROOTINO)
+   vp->v_flag |= VROOT;
+   /*
+* Initialize modrev times
+*/
+   getmicrouptime();
+   ip->i_modrev = (u_quad_t)mtv.tv_sec << 32;
+   ip->i_modrev |= (u_quad_t)mtv.tv_usec * 4294;
+   *vpp = vp;
+   return (0);
+}
Index: ufs/ffs/ffs_vfsops.c
===
RCS file: /cvs/src/sys/ufs/ffs/ffs_vfsops.c,v
retrieving revision 1.160
diff -u -p -r1.160 ffs_vfsops.c
--- ufs/ffs/ffs_vfsops.c19 Jun 2016 11:54:34 -  1.160
+++ ufs/ffs/ffs_vfsops.c7 Aug 2016 21:25:39 -
@@ -1367,8 +1367,7 @@ retry:
 * Initialize the vnode from the inode, check for aliases.
 * Note that the underlying vnode may have changed.
 */
-   error = ufs_vinit(mp, _specvops, FFS_FIFOOPS, );
-   if (error) {
+   if ((error = ffs_vinit(mp, )) != 0) {
vput(vp);
*vpp = NULL;
return (error);
Index: ufs/ufs/ufs_extern.h
===
RCS file: /cvs/src/sys/ufs/ufs/ufs_extern.h,v
retrieving revision 1.35
diff -u -p -r1.35 ufs_extern.h
--- ufs/ufs/ufs_extern.h25 Jan 2014 23:31:13 -  1.35
+++ ufs/ufs/ufs_extern.h7 Aug 2016 21:25:39 -
@@ -130,7 +130,6 @@ int ufs_check_export(struct mount *, str
struct ucred **);
 
 /* ufs_vnops.c */
-int ufs_vinit(struct mount *, struct vops *, struct vops *, struct vnode **);
 void ufs_itimes(struct vnode *);
 int ufs_makeinode(int, struct vnode *, struct vnode **,
  struct componentname *);
Index: ufs/ufs/ufs_vnops.c
===

ext2fs_vinit() cleanup

[Martin Natano]

ext2fs has only one set of specops/fifoops, so no need to pass those to
the function. This also allows to get rid of the EXT2FS_FIFOOPS define.

Ok?

natano


Index: ufs/ext2fs/ext2fs_extern.h
===
RCS file: /cvs/src/sys/ufs/ext2fs/ext2fs_extern.h,v
retrieving revision 1.35
diff -u -p -r1.35 ext2fs_extern.h
--- ufs/ext2fs/ext2fs_extern.h  14 Jul 2014 08:54:13 -  1.35
+++ ufs/ext2fs/ext2fs_extern.h  7 Aug 2016 20:54:43 -
@@ -91,8 +91,7 @@ int   ext2fs_checkpath(struct inode *, str
 
 /* ext2fs_subr.c */
 intext2fs_bufatoff(struct inode *, off_t, char **, struct buf **);
-intext2fs_vinit(struct mount *, struct vops *, struct vops *,
-   struct vnode **);
+intext2fs_vinit(struct mount *, struct vnode **);
 #ifdef DIAGNOSTIC
 void   ext2fs_checkoverlap(struct buf *, struct inode *);
 #endif
@@ -149,7 +148,4 @@ extern struct vops ext2fs_vops;
 extern struct vops ext2fs_specvops;
 #ifdef FIFO
 extern struct vops ext2fs_fifovops;
-#define EXT2FS_FIFOOPS _fifovops
-#else
-#define EXT2FS_FIFOOPS NULL
 #endif
Index: ufs/ext2fs/ext2fs_subr.c
===
RCS file: /cvs/src/sys/ufs/ext2fs/ext2fs_subr.c,v
retrieving revision 1.34
diff -u -p -r1.34 ext2fs_subr.c
--- ufs/ext2fs/ext2fs_subr.c4 Feb 2016 12:45:03 -   1.34
+++ ufs/ext2fs/ext2fs_subr.c7 Aug 2016 20:54:43 -
@@ -145,8 +145,7 @@ ext2fs_checkoverlap(struct buf *bp, stru
  * Initialize the vnode associated with a new inode, handle aliased vnodes.
  */
 int
-ext2fs_vinit(struct mount *mp, struct vops *specops,
-struct vops *fifoops, struct vnode **vpp)
+ext2fs_vinit(struct mount *mp, struct vnode **vpp)
 {
struct inode *ip;
struct vnode *vp, *nvp;
@@ -159,7 +158,7 @@ ext2fs_vinit(struct mount *mp, struct vo
switch(vp->v_type) {
case VCHR:
case VBLK:
-   vp->v_op = specops;
+   vp->v_op = _specvops;
 
nvp = checkalias(vp, letoh32(ip->i_e2din->e2di_rdev), mp);
if (nvp != NULL) {
@@ -185,7 +184,7 @@ ext2fs_vinit(struct mount *mp, struct vo
 
case VFIFO:
 #ifdef FIFO
-   vp->v_op = fifoops;
+   vp->v_op = _fifovops;
break;
 #else
return (EOPNOTSUPP);
Index: ufs/ext2fs/ext2fs_vfsops.c
===
RCS file: /cvs/src/sys/ufs/ext2fs/ext2fs_vfsops.c,v
retrieving revision 1.93
diff -u -p -r1.93 ext2fs_vfsops.c
--- ufs/ext2fs/ext2fs_vfsops.c  19 Jun 2016 11:54:33 -  1.93
+++ ufs/ext2fs/ext2fs_vfsops.c  7 Aug 2016 20:54:43 -
@@ -918,7 +918,7 @@ ext2fs_vget(struct mount *mp, ino_t ino,
 * Initialize the vnode from the inode, check for aliases.
 * Note that the underlying vnode may have changed.
 */
-   error = ext2fs_vinit(mp, _specvops, EXT2FS_FIFOOPS, );
+   error = ext2fs_vinit(mp, );
if (error) {
vput(vp);
*vpp = NULL;

Re: hotplugd restart problem

[Martin Natano]

On Mon, Jul 25, 2016 at 02:17:11PM +0300, Alexey Vatchenko wrote:
> Hi!
> If something is running which is started from attach script, restarting of
> hotplugd fails because /dev/hotplug is occupied by child process.
> 
> Attached is the patch to fix this problem.

Makes sense, but why not pass O_CLOEXEC to open() instead of using
fcntl()?


> Index: usr.sbin/hotplugd/hotplugd.c
> ===
> RCS file: /cvs/src/usr.sbin/hotplugd/hotplugd.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 hotplugd.c
> --- usr.sbin/hotplugd/hotplugd.c  19 Nov 2015 06:05:40 -  1.13
> +++ usr.sbin/hotplugd/hotplugd.c  25 Jul 2016 11:05:12 -
> @@ -82,6 +82,7 @@ main(int argc, char *argv[])
>  
>   if ((devfd = open(device, O_RDONLY)) == -1)
>   err(1, "%s", device);
> + fcntl(devfd, F_SETFD, FD_CLOEXEC);
>  
>   bzero(, sizeof(sact));
>   sigemptyset(_mask);

Re: [Bug49] Tmpfs mount with bad args can lead to a panic

[Martin Natano]

On Mon, Jul 11, 2016 at 04:39:05PM -0400, Ted Unangst wrote:
> Tim Newsham wrote:
> > The tmpfs filesystem allows the mounting user to specify a
> > username, a groupname or a device name for the root node of
> > the filesystem.  A user that specifies a value of VNOVAL for
> > any of these fields will trigger an assert in tmpfs_alloc_node():
> > 
> > /* XXX pedro: we should check for UID_MAX and GID_MAX instead. */
> > KASSERT(uid != VNOVAL && gid != VNOVAL && mode != VNOVAL);
> 
> sigh. i don't know what else can trigger that kassert, so just fix the caller
> to do the same check and return an error.
> 
> Index: tmpfs_vfsops.c
> ===
> RCS file: /cvs/src/sys/tmpfs/tmpfs_vfsops.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 tmpfs_vfsops.c
> --- tmpfs_vfsops.c13 Jan 2016 13:01:40 -  1.8
> +++ tmpfs_vfsops.c11 Jul 2016 20:37:30 -
> @@ -125,6 +125,9 @@ tmpfs_mount(struct mount *mp, const char
>   error = copyin(data, , sizeof(struct tmpfs_args));
>   if (error)
>   return error;
> + if (args.ta_root_uid == VNOVAL || args.ta_root_gid == VNOVAL ||
> + (args.ta_root_mode & ALLPERMS) == VNOVAL)
> + return EINVAL;

ta_root_mode contains the S_IFDIR format bit (not included in ALLPERMS)
when using mount_tmpfs(8) with the default arguments.

With the last check changed to (args.ta_root_mode == VNOVAL), OK natano@.


>  
>   /* Get the memory usage limit for this file-system. */
>   if (args.ta_size_max < PAGE_SIZE) {
> 

Re: [Bug49] Tmpfs mount with bad args can lead to a panic

[Martin Natano]

On Mon, Jul 11, 2016 at 05:06:33PM -0400, Ted Unangst wrote:
> Todd C. Miller wrote:
> > On Mon, 11 Jul 2016 16:39:05 -0400, "Ted Unangst" wrote:
> > 
> > > sigh. i don't know what else can trigger that kassert, so just fix the 
> > > caller
> > > to do the same check and return an error.
> > 
> > Checking for VNOVAL is kind of bogus.  How about we try something
> > more sensible?
> 
> those checks are equally useless. UID_MAX is UINT_MAX so the tests don't fire.
> 
> the question is what other tmpfs code blows up when nodes owned by -1 start
> showing up.

The check for VNOVAl is probably the right thing to do here. Using
VNOVAL to indicate the absence of a value is an established (although
rather questionable) practice in filesystem code. e.g. a VOP_SETATTR()
call with vap->va_uid set to VNOVAL will not chown the file, making
VNOVAL a de facto illegal value for a file owner.


> 
> > 
> >  - todd
> > 
> > Index: tmpfs_subr.c
> > ===
> > RCS file: /cvs/src/sys/tmpfs/tmpfs_subr.c,v
> > retrieving revision 1.16
> > diff -u -p -u -r1.16 tmpfs_subr.c
> > --- tmpfs_subr.c19 Jun 2016 11:54:33 -  1.16
> > +++ tmpfs_subr.c11 Jul 2016 20:45:26 -
> > @@ -139,8 +139,7 @@ tmpfs_alloc_node(tmpfs_mount_t *tmp, enu
> > nnode->tn_ctime = nnode->tn_atime;
> > nnode->tn_mtime = nnode->tn_atime;
> >  
> > -   /* XXX pedro: we should check for UID_MAX and GID_MAX instead. */
> > -   KASSERT(uid != VNOVAL && gid != VNOVAL && mode != VNOVAL);
> > +   KASSERT(uid <= UID_MAX && gid <= GID_MAX && (mode & ALLPERMS) == mode);
> >  
> > nnode->tn_uid = uid;
> > nnode->tn_gid = gid;
> > Index: tmpfs_vfsops.c
> > ===
> > RCS file: /cvs/src/sys/tmpfs/tmpfs_vfsops.c,v
> > retrieving revision 1.8
> > diff -u -p -u -r1.8 tmpfs_vfsops.c
> > --- tmpfs_vfsops.c  13 Jan 2016 13:01:40 -  1.8
> > +++ tmpfs_vfsops.c  11 Jul 2016 20:45:20 -
> > @@ -126,6 +126,11 @@ tmpfs_mount(struct mount *mp, const char
> > if (error)
> > return error;
> >  
> > +   /* Sanity check args. */
> > +   if (args.ta_root_uid > UID_MAX || args.ta_root_gid > GID_MAX ||
> > +   (args.ta_root_mode & ALLPERMS) != args.ta_root_mode)
> > +   return EINVAL;
> > +
> > /* Get the memory usage limit for this file-system. */
> > if (args.ta_size_max < PAGE_SIZE) {
> > memlimit = UINT64_MAX;
> > 
> 

Re: nfsrv_mknod(): missing vput() in error path

[Martin Natano]

FWIW, this is the diff I used to simulate the race condition (only for
testing):

Index: kern/vfs_syscalls.c
===
RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.260
diff -u -p -r1.260 vfs_syscalls.c
--- kern/vfs_syscalls.c 3 Jul 2016 04:36:08 -   1.260
+++ kern/vfs_syscalls.c 3 Jul 2016 19:09:24 -
@@ -1233,9 +1233,12 @@ domknodat(struct proc *p, int fd, const 
if ((error = namei()) != 0)
return (error);
vp = nd.ni_vp;
-   if (vp != NULL)
-   error = EEXIST;
-   else {
+   if (vp != NULL) {
+   /* error = EEXIST; */
+   error = 0;
+   vrele(vp);
+   }
+   /* else { */
VATTR_NULL();
vattr.va_mode = (mode & ALLPERMS) &~ p->p_fd->fd_cmask;
if ((p->p_p->ps_flags & PS_PLEDGE))
@@ -1267,7 +1270,7 @@ domknodat(struct proc *p, int fd, const 
error = EINVAL;
break;
}
-   }
+   /* } */
if (!error) {
error = VOP_MKNOD(nd.ni_dvp, _vp, _cnd, );
} else {


On Sun, Jul 03, 2016 at 09:16:26PM +0200, Martin Natano wrote:
> When perfoming a mknod operation over NFS it can happen that another
> client creates a file with the same name between the namei() and
> VOP_MKNOD() calls in domknod(). This leads to an error path in
> nfsrv_mknod() being triggered. In that error path there is a
> vput(nd.ni_np) missing, resulting in the vnode getting stuck with a
> stale reference and lock, while it shouldn't have either.
> 
> Ok?
> 
> natano
> 
> 
> Index: nfs/nfs_serv.c
> ===
> RCS file: /cvs/src/sys/nfs/nfs_serv.c,v
> retrieving revision 1.108
> diff -u -p -r1.108 nfs_serv.c
> --- nfs/nfs_serv.c29 Apr 2016 14:40:36 -  1.108
> +++ nfs/nfs_serv.c3 Jul 2016 18:00:54 -
> @@ -1163,7 +1163,12 @@ nfsrv_mknod(struct nfsrv_descript *nfsd,
>   pool_put(_pool, nd.ni_cnd.cn_pnbuf);
>   error = NFSERR_BADTYPE;
>   VOP_ABORTOP(nd.ni_dvp, _cnd);
> - vput(nd.ni_dvp);
> + if (nd.ni_dvp == nd.ni_vp)
> + vrele(nd.ni_dvp);
> + else
> + vput(nd.ni_dvp);
> + if (nd.ni_vp)
> + vput(nd.ni_vp);
>   goto out;
>   }
>   VATTR_NULL();
> @@ -1185,7 +1190,11 @@ nfsrv_mknod(struct nfsrv_descript *nfsd,
>   pool_put(_pool, nd.ni_cnd.cn_pnbuf);
>   error = EEXIST;
>   VOP_ABORTOP(nd.ni_dvp, _cnd);
> - vput(nd.ni_dvp);
> + if (nd.ni_dvp == nd.ni_vp)
> + vrele(nd.ni_dvp);
> + else
> + vput(nd.ni_dvp);
> + vput(nd.ni_vp);
>   goto out;
>   }
>   va.va_type = vtyp;

nfsrv_mknod(): missing vput() in error path

[Martin Natano]

When perfoming a mknod operation over NFS it can happen that another
client creates a file with the same name between the namei() and
VOP_MKNOD() calls in domknod(). This leads to an error path in
nfsrv_mknod() being triggered. In that error path there is a
vput(nd.ni_np) missing, resulting in the vnode getting stuck with a
stale reference and lock, while it shouldn't have either.

Ok?

natano


Index: nfs/nfs_serv.c
===
RCS file: /cvs/src/sys/nfs/nfs_serv.c,v
retrieving revision 1.108
diff -u -p -r1.108 nfs_serv.c
--- nfs/nfs_serv.c  29 Apr 2016 14:40:36 -  1.108
+++ nfs/nfs_serv.c  3 Jul 2016 18:00:54 -
@@ -1163,7 +1163,12 @@ nfsrv_mknod(struct nfsrv_descript *nfsd,
pool_put(_pool, nd.ni_cnd.cn_pnbuf);
error = NFSERR_BADTYPE;
VOP_ABORTOP(nd.ni_dvp, _cnd);
-   vput(nd.ni_dvp);
+   if (nd.ni_dvp == nd.ni_vp)
+   vrele(nd.ni_dvp);
+   else
+   vput(nd.ni_dvp);
+   if (nd.ni_vp)
+   vput(nd.ni_vp);
goto out;
}
VATTR_NULL();
@@ -1185,7 +1190,11 @@ nfsrv_mknod(struct nfsrv_descript *nfsd,
pool_put(_pool, nd.ni_cnd.cn_pnbuf);
error = EEXIST;
VOP_ABORTOP(nd.ni_dvp, _cnd);
-   vput(nd.ni_dvp);
+   if (nd.ni_dvp == nd.ni_vp)
+   vrele(nd.ni_dvp);
+   else
+   vput(nd.ni_dvp);
+   vput(nd.ni_vp);
goto out;
}
va.va_type = vtyp;

Re: continue in empty loops

[Martin Natano]

On Wed, Jun 01, 2016 at 01:08:05PM -0400, Ted Unangst wrote:
> I find this easier to read. My old eyes don't focus on the semicolon, which
> makes me wonder what's supposed to be happening.

Yes, please! ok natano@


> 
> 
> Index: pax/ar_io.c
> ===
> RCS file: /cvs/src/bin/pax/ar_io.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 ar_io.c
> --- pax/ar_io.c   6 Dec 2015 16:57:45 -   1.55
> +++ pax/ar_io.c   1 Jun 2016 17:05:32 -
> @@ -427,7 +427,7 @@ ar_drain(void)
>* keep reading until pipe is drained
>*/
>   while ((res = read(arfd, drbuf, sizeof(drbuf))) > 0)
> - ;
> + continue;
>   lstrval = res;
>  }
>  
> @@ -1038,7 +1038,7 @@ get_phys(void)
>* (this is a bit paranoid, but should be safe to do).
>*/
>   while ((res = read(arfd, scbuf, sizeof(scbuf))) > 0)
> - ;
> + continue;
>   if (res < 0) {
>   syswarn(1, errno, "Unable to locate tape filemark.");
>   return(-1);
> Index: pax/ftree.c
> ===
> RCS file: /cvs/src/bin/pax/ftree.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 ftree.c
> --- pax/ftree.c   19 Mar 2015 05:14:24 -  1.38
> +++ pax/ftree.c   1 Jun 2016 17:05:38 -
> @@ -548,7 +548,7 @@ getpathname(char *buf, int buflen)
>   term = '\n';
>   }
>   while ((ch = getchar()) != term && ch != EOF)
> - ;
> + continue;
>   paxwarn(1, "Ignoring too-long pathname: %s", buf);
>   return(NULL);
>  }
> Index: rmdir/rmdir.c
> ===
> RCS file: /cvs/src/bin/rmdir/rmdir.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 rmdir.c
> --- rmdir/rmdir.c 9 Oct 2015 01:37:06 -   1.11
> +++ rmdir/rmdir.c 1 Jun 2016 17:05:58 -
> @@ -75,7 +75,7 @@ main(int argc, char *argv[])
>   /* Delete trailing slashes, per POSIX. */
>   p = *argv + strlen(*argv);
>   while (--p > *argv && *p == '/')
> - ;
> + continue;
>   *++p = '\0';
>  
>   if (rmdir(*argv) < 0) {
> @@ -96,7 +96,7 @@ rm_path(char *path)
>   while ((p = strrchr(path, '/')) != NULL) {
>   /* Delete trailing slashes. */
>   while (--p > path && *p == '/')
> - ;
> + continue;
>   *++p = '\0';
>  
>   if (rmdir(path) < 0) {
> 

Re: ancient relics in newsyslog

[Martin Natano]

On Wed, Jun 01, 2016 at 01:43:06AM -0400, Ted Unangst wrote:
> Let's make the defaults be the defaults.

Reads fine. ok natano@


> 
> Index: Makefile
> ===
> RCS file: /cvs/src/usr.bin/newsyslog/Makefile,v
> retrieving revision 1.6
> diff -u -p -r1.6 Makefile
> --- Makefile  30 Mar 2016 06:38:46 -  1.6
> +++ Makefile  1 Jun 2016 05:42:06 -
> @@ -2,14 +2,6 @@
>  
>  PROG=newsyslog
>  
> -CFLAGS+= -DCONF=\"/etc/newsyslog.conf\"
> -CFLAGS+= -DPIDFILE=\"/var/run/syslog.pid\"
> -CFLAGS+= -DCOMPRESS=\"/usr/bin/gzip\"
> -CFLAGS+= -DCOMPRESS_POSTFIX=\".gz\"
> -CFLAGS+= -DSTATS_DIR=\"/var/run\"
> -CFLAGS+= -DSENDMAIL=\"/usr/sbin/sendmail\"
> -CFLAGS+= -DQUAD_OFF_T
> -
>  BINOWN=  root
>  
>  MAN= newsyslog.8
> Index: newsyslog.c
> ===
> RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.c,v
> retrieving revision 1.100
> diff -u -p -r1.100 newsyslog.c
> --- newsyslog.c   11 Jan 2016 19:26:04 -  1.100
> +++ newsyslog.c   1 Jun 2016 05:41:59 -
> @@ -71,24 +71,12 @@
>   *
>   */
>  
> -#ifndef CONF
> -#define CONF "/etc/newsyslog.conf" /* Configuration file */
> -#endif
> -#ifndef PIDFILE
> -#define PIDFILE "/etc/syslog.pid"
> -#endif
> -#ifndef COMPRESS
> -#define COMPRESS "/usr/bin/compress" /* File compression program */
> -#endif
> -#ifndef COMPRESS_POSTFIX
> -#define COMPRESS_POSTFIX ".Z"
> -#endif
> -#ifndef STATS_DIR
> -#define STATS_DIR "/etc"
> -#endif
> -#ifndef SENDMAIL
> -#define SENDMAIL "/usr/lib/sendmail"
> -#endif
> +#define CONF "/etc/newsyslog.conf"
> +#define PIDFILE "/var/run/syslog.pid"
> +#define COMPRESS "/usr/bin/gzip"
> +#define COMPRESS_POSTFIX ".gz"
> +#define STATS_DIR "/var/run"
> +#define SENDMAIL "/usr/sbin/sendmail"
>  
>  #include/* DEV_BSIZE */
>  #include 
> @@ -1002,11 +990,7 @@ domonitor(struct conf_entry *ent)
>   warn("%s", fname);
>   goto cleanup;
>   }
> -#ifdef QUAD_OFF_T
>   if (fscanf(fp, "%lld\n", ) != 1) {
> -#else
> - if (fscanf(fp, "%ld\n", ) != 1) {
> -#endif   /* QUAD_OFF_T */
>   fclose(fp);
>   goto update;
>   }
> @@ -1060,11 +1044,7 @@ update:
>   warn("%s", fname);
>   goto cleanup;
>   }
> -#ifdef QUAD_OFF_T
>   fprintf(fp, "%lld\n", (long long)sb.st_size);
> -#else
> - fprintf(fp, "%ld\n", (long)sb.st_size);
> -#endif   /* QUAD_OFF_T */
>   fclose(fp);
>  
>  cleanup:
> 

ntfs mkdir errno value

[Martin Natano]

The following patch addresses an issue found by landry. mkdir on an ntfs
mountpoint returns ENOENT, while the error should be EROFS. We don't
support write access on ntfs.

Ok?

natano


Index: ntfs/ntfs_subr.c
===
RCS file: /cvs/src/sys/ntfs/ntfs_subr.c,v
retrieving revision 1.45
diff -u -p -r1.45 ntfs_subr.c
--- ntfs/ntfs_subr.c7 Feb 2016 09:31:14 -   1.45
+++ ntfs/ntfs_subr.c31 May 2016 19:25:42 -
@@ -1055,14 +1055,20 @@ ntfs_ntlookupfile(struct ntfsmount *ntmp
}
} while (1);
 
-   /* perform full scan if no entry was found */
-   if (!fullscan && error == ENOENT) {
-   fullscan = 1;
-   cn = 0; /* need zero, used by lookup_ctx */
+   if (error == ENOENT) {
+   /* perform full scan if no entry was found */
+   if (!fullscan) {
+   fullscan = 1;
+   cn = 0; /* need zero, used by lookup_ctx */
 
-   DDPRINTF("ntfs_ntlookupfile: fullscan performed for: %.*s\n",
-   (unsigned int)fnamelen, fname);
-   goto loop;
+   DDPRINTF("ntfs_ntlookupfile: fullscan performed for: 
%.*s\n",
+   (unsigned int)fnamelen, fname);
+   goto loop;
+   }
+
+   if ((cnp->cn_flags & ISLASTCN) &&
+   (cnp->cn_nameiop == CREATE || cnp->cn_nameiop == RENAME))
+   error = EJUSTRETURN;
}
 
DPRINTF("finish\n");

lockmgr() api removal

[Martin Natano]

It is time for the lockmgr() api to die. The api is only used by
filesystems, where it is a trivial change to use rrw locks instead. All
it needs is LK_* defines for the RW_* flags. (See the sys/lock.h hunk in
the diff below.)

The ffs regress tests display the same number of fail/ok results before
and after applying diff below, and I have done some manual testing with
various filesystems on amd64 and macppc.

Again, the purpose is to make filesystem code less scary and more
comprehensible.

Ok?

natano


Index: distrib/sets/lists/comp/mi
===
RCS file: /cvs/src/distrib/sets/lists/comp/mi,v
retrieving revision 1.1233
diff -u -p -r1.1233 mi
--- distrib/sets/lists/comp/mi  23 May 2016 00:59:55 -  1.1233
+++ distrib/sets/lists/comp/mi  29 May 2016 18:13:57 -
@@ -2712,7 +2712,6 @@
 ./usr/share/man/man9/kthread.9
 ./usr/share/man/man9/ktrace.9
 ./usr/share/man/man9/loadfirmware.9
-./usr/share/man/man9/lock.9
 ./usr/share/man/man9/log.9
 ./usr/share/man/man9/malloc.9
 ./usr/share/man/man9/mbuf.9
Index: share/man/man9/Makefile
===
RCS file: /cvs/src/share/man/man9/Makefile,v
retrieving revision 1.276
diff -u -p -r1.276 Makefile
--- share/man/man9/Makefile 25 Apr 2016 19:24:42 -  1.276
+++ share/man/man9/Makefile 29 May 2016 18:14:11 -
@@ -20,7 +20,7 @@ MAN=  aml_evalnode.9 atomic_add_int.9 ato
ieee80211_radiotap.9 if_get.9 if_rxr_init.9 ifq_enqueue.9 \
ifq_deq_begin.9 iic.9 intro.9 inittodr.9 intr_barrier.9 \
kern.9 km_alloc.9 knote.9 kthread.9 ktrace.9 \
-   loadfirmware.9 lock.9 log.9 \
+   loadfirmware.9 log.9 \
malloc.9 membar_sync.9 mbuf.9 mbuf_tags.9 md5.9 mi_switch.9 \
microtime.9 ml_init.9 mq_init.9 mutex.9 \
namei.9 \
Index: share/man/man9/VOP_LOOKUP.9
===
RCS file: /cvs/src/share/man/man9/VOP_LOOKUP.9,v
retrieving revision 1.35
diff -u -p -r1.35 VOP_LOOKUP.9
--- share/man/man9/VOP_LOOKUP.9 23 May 2016 09:31:28 -  1.35
+++ share/man/man9/VOP_LOOKUP.9 29 May 2016 18:14:11 -
@@ -1,6 +1,7 @@
 .\" $OpenBSD: VOP_LOOKUP.9,v 1.35 2016/05/23 09:31:28 natano Exp $
 .\"
 .\" Copyright (c) 2003 Ted Unangst
+.\" Copyright (c) 2000, 2001 The NetBSD Foundation, Inc.
 .\" All rights reserved.
 .\"
 .\" Redistribution and use in source and binary forms, with or without
@@ -565,16 +566,54 @@ to lock a vnode.
 It should not be used by other file system code.
 .Fn VOP_UNLOCK
 unlocks a vnode.
-.Fn VOP_ISLOCKED
-returns 1 if
-.Fa vp
-is locked and 0 if not.
-It should be used cautiously, as not all file systems implement locks
-effectively.
 Note the asymmetry between
 .Xr vn_lock 9
 and
 .Fn VOP_UNLOCK .
+.Pp
+.Fa flags
+may contain the following flags:
+.Pp
+.Bl -tag -width LK_RECURSEFAIL -compact -offset indent
+.It Dv LK_EXCLUSIVE
+Acquire an exclusive lock.
+.It Dv LK_SHARED
+Acquire a shared lock.
+.It Dv LK_NOWAIT
+Don't wait if the vnode lock is held by someone else
+(may still wait on reclamation lock).
+.It Dv LK_RECURSEFAIL
+Attempt at recursive lock fails.
+.It Dv LK_DRAIN
+Wait for all activity on the lock to end, then mark it decommissioned.
+This feature is used to ensure that no other activity can occur while the
+underlying object of a vnode is being cleaned out.
+Must be used in combination with
+.Dv LK_EXCLUSIVE .
+.El
+.Pp
+.Fn VOP_ISLOCKED
+returns one of the following values:
+.Pp
+.Bl -tag -width LK_EXCLUSIVE -compact -offset indent
+.It Dv LK_EXCLUSIVE
+.Fa vp
+is locked for exclusive access by the calling thread.
+.It Dv LK_EXCLOTHER
+.Fa vp
+is locked for exclusive access by a different thread.
+.It Dv LK_SHARED
+.Fa vp
+is locked for shared access.
+The current thread may be one of the threads that have it locked.
+.It 0
+.Fa vp
+is not locked.
+.El
+.Pp
+.Fn VOP_ISLOCKED
+should be used cautiously, as not all file systems implement locks
+effectively.
 .Pp
 .It Fn VOP_KQFILTER vp kn
 Register the
Index: share/man/man9/mutex.9
===
RCS file: /cvs/src/share/man/man9/mutex.9,v
retrieving revision 1.22
diff -u -p -r1.22 mutex.9
--- share/man/man9/mutex.9  13 Feb 2014 14:23:05 -  1.22
+++ share/man/man9/mutex.9  29 May 2016 18:14:11 -
@@ -104,7 +104,6 @@ function will return non-zero if it succ
 .Fa mtxp ,
 otherwise it will return 0.
 .Sh SEE ALSO
-.Xr lockmgr 9 ,
 .Xr msleep 9 ,
 .Xr rwlock 9 ,
 .Xr spl 9
Index: share/man/man9/rwlock.9
===
RCS file: /cvs/src/share/man/man9/rwlock.9,v
retrieving revision 1.17
diff -u -p -r1.17 rwlock.9
--- share/man/man9/rwlock.9 9 Jul 2014 18:00:09 -   1.17
+++ share/man/man9/rwlock.9 29 May 2016 18:14:11 -
@@ -183,8 +183,8 @@ can be called during autoconf, from proc
 .Pp
 All other functions can be called during 

Re: libc: delete unused hash algorithms

[Martin Natano]

On Sat, May 28, 2016 at 07:47:50PM -0700, Philip Guenther wrote:
> 
> Overriding the hash algorithm used by the Berkeley DB bits isn't support 
> (it would break getpw* if nothing else) and hasn't been possible since the 
> symbol hiding effort last fall.  So eliminate the redirection through a 
> variable and declare it as a hidden function to eliminate the relocations 
> for it.
> 
> Ok?

Looks good to me. libc shouldn't be a dumping ground for everyone's
favorite hash function, especially if not even compiled in.


> 
> 
> Philip Guenther
> 
> 
> Index: hidden/db.h
> ===
> RCS file: /data/src/openbsd/src/lib/libc/hidden/db.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 db.h
> --- hidden/db.h   17 Oct 2015 21:48:42 -  1.3
> +++ hidden/db.h   20 May 2016 09:03:13 -
> @@ -73,6 +73,9 @@ DB  *__bt_open(const char *, int, int, co
>  DB   *__hash_open(const char *, int, int, const HASHINFO *, int);
>  DB   *__rec_open(const char *, int, int, const RECNOINFO *, int);
>  void __dbpanic(DB *dbp);
> +
> +/* Default hash function, from db/hash/hash_func.c */
> +u_int32_t__default_hash(const void *, size_t);
>  __END_HIDDEN_DECLS
>  
>  PROTO_NORMAL(dbopen);
> Index: stdlib/hcreate.c
> ===
> RCS file: /data/src/openbsd/src/lib/libc/stdlib/hcreate.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 hcreate.c
> --- stdlib/hcreate.c  10 Sep 2015 18:13:46 -  1.6
> +++ stdlib/hcreate.c  20 May 2016 09:03:55 -
> @@ -55,6 +55,8 @@
>  #include 
>  #include 
>  
> +#include   /* for __default_hash */
> +
>  #ifndef _DIAGASSERT
>  #define _DIAGASSERT(x)
>  #endif
> @@ -79,9 +81,6 @@ SLIST_HEAD(internal_head, internal_entry
>  #define  MAX_BUCKETS_LG2 (sizeof (size_t) * 8 - 1 - 5)
>  #define  MAX_BUCKETS ((size_t)1 << MAX_BUCKETS_LG2)
>  
> -/* Default hash function, from db/hash/hash_func.c */
> -extern u_int32_t (*__default_hash)(const void *, size_t);
> -
>  static struct internal_head *htable;
>  static size_t htablesize;
>  
> @@ -164,7 +163,7 @@ hsearch(ENTRY item, ACTION action)
>   _DIAGASSERT(action == ENTER || action == FIND);
>  
>   len = strlen(item.key);
> - hashval = (*__default_hash)(item.key, len);
> + hashval = __default_hash(item.key, len);
>  
>   head = [hashval & (htablesize - 1)];
>   ie = SLIST_FIRST(head);
> Index: db/hash/extern.h
> ===
> RCS file: /data/src/openbsd/src/lib/libc/db/hash/extern.h,v
> retrieving revision 1.8
> diff -u -p -r1.8 extern.h
> --- db/hash/extern.h  27 Aug 2015 04:37:09 -  1.8
> +++ db/hash/extern.h  20 May 2016 09:04:27 -
> @@ -56,9 +56,6 @@ int  __put_page(HTAB *, char *, u_int32_
>  void  __reclaim_buf(HTAB *, BUFHEAD *);
>  int   __split_page(HTAB *, u_int32_t, u_int32_t);
>  
> -/* Default hash routine. */
> -extern u_int32_t (*__default_hash)(const void *, size_t);
> -
>  #ifdef HASH_STATISTICS
>  extern int hash_accesses, hash_collisions, hash_expansions, hash_overflows;
>  #endif
> Index: db/hash/hash_func.c
> ===
> RCS file: /data/src/openbsd/src/lib/libc/db/hash/hash_func.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 hash_func.c
> --- db/hash/hash_func.c   5 Aug 2005 13:03:00 -   1.10
> +++ db/hash/hash_func.c   29 May 2016 02:41:12 -
> @@ -35,118 +35,10 @@
>  #include 
>  
>  #include 
> -#include "hash.h"
> -#include "page.h"
> -#include "extern.h"
> -
> -#ifdef notdef
> -static u_int32_t hash1(const void *, size_t);
> -static u_int32_t hash2(const void *, size_t);
> -static u_int32_t hash3(const void *, size_t);
> -#endif
> -static u_int32_t hash4(const void *, size_t);
> -
> -/* Default hash function. */
> -u_int32_t (*__default_hash)(const void *, size_t) = hash4;
> -
> -#ifdef notdef
> -/*
> - * Assume that we've already split the bucket to which this key hashes,
> - * calculate that bucket, and check that in fact we did already split it.
> - *
> - * EJB's original hsearch hash.
> - */
> -#define PRIME1   37
> -#define PRIME2   1048583
> -
> -u_int32_t
> -hash1(const void *key, size_t len)
> -{
> - u_int32_t h;
> - u_int8_t *k;
> -
> - h = 0;
> - k = (u_int8_t *)key;
> - /* Convert string to integer */
> - while (len--)
> - h = h * PRIME1 ^ (*k++ - ' ');
> - h %= PRIME2;
> - return (h);
> -}
> -
> -/*
> - * Phong Vo's linear congruential hash
> - */
> -#define dcharhash(h, c)  ((h) = 0x63c63cd9*(h) + 0x9c39c33d + (c))
> -
> -u_int32_t
> -hash2(const void *key, size_t len)
> -{
> - u_int32_t h;
> - u_int8_t *e, c, *k;
> -
> - k = (u_int8_t *)key;
> - e = k + len;
> - for (h = 0; k != e;) {
> - c = *k++;
> - if (!c && k > e)
> - break;
> - 

Re: prefer AF_* over PF_*

[Martin Natano]

On Sat, May 28, 2016 at 07:55:00PM -0700, Philip Guenther wrote:
> 
> About the only place userland code should use PF_* socket constants is 
> with sysctl(3)'s CTL_NET hierarchy.  All the standardized functions are 
> defined as taking AF_* values.  Let's use the preferred names in the 
> getaddrinfo(3) and socketpair(2) manpages.
> 
> ok?

POSIX only mentions AF_*. ok natano@


> 
> Philip Guenther
> 
> Index: net/getaddrinfo.3
> ===
> RCS file: /data/src/openbsd/src/lib/libc/net/getaddrinfo.3,v
> retrieving revision 1.57
> diff -u -p -r1.57 getaddrinfo.3
> --- net/getaddrinfo.3 16 Feb 2015 18:26:56 -  1.57
> +++ net/getaddrinfo.3 20 May 2016 00:06:21 -
> @@ -94,7 +94,7 @@ The protocol family that should be used.
>  When
>  .Fa ai_family
>  is set to
> -.Dv PF_UNSPEC ,
> +.Dv AF_UNSPEC ,
>  it means the caller will accept any protocol family supported by the
>  operating system.
>  .It Fa ai_socktype
> @@ -229,7 +229,7 @@ behaves as if the caller provided a
>  with
>  .Fa ai_family
>  set to
> -.Dv PF_UNSPEC ,
> +.Dv AF_UNSPEC ,
>  .Fa ai_flags
>  set to
>  .Dv AI_ADDRCONFIG ,
> @@ -351,7 +351,7 @@ int s;
>  const char *cause = NULL;
>  
>  memset(, 0, sizeof(hints));
> -hints.ai_family = PF_UNSPEC;
> +hints.ai_family = AF_UNSPEC;
>  hints.ai_socktype = SOCK_STREAM;
>  error = getaddrinfo("www.kame.net", "www", , );
>  if (error)
> @@ -393,7 +393,7 @@ int nsock;
>  const char *cause = NULL;
>  
>  memset(, 0, sizeof(hints));
> -hints.ai_family = PF_UNSPEC;
> +hints.ai_family = AF_UNSPEC;
>  hints.ai_socktype = SOCK_STREAM;
>  hints.ai_flags = AI_PASSIVE;
>  error = getaddrinfo(NULL, "www", , );
> Index: sys/socketpair.2
> ===
> RCS file: /data/src/openbsd/src/lib/libc/sys/socketpair.2,v
> retrieving revision 1.19
> diff -u -p -r1.19 socketpair.2
> --- sys/socketpair.2  19 Mar 2016 22:10:49 -  1.19
> +++ sys/socketpair.2  20 May 2016 00:05:48 -
> @@ -124,7 +124,7 @@ This call is currently implemented only 
>  Many operating systems only accept a
>  .Fa protocol
>  of
> -.Dv PF_UNSPEC ,
> +.Dv AF_UNSPEC ,
>  so that should be used instead of
> -.Dv PF_LOCAL
> +.Dv AF_LOCAL
>  for maximal portability.
> 

Re: ptrace PT_IO write bug

[Martin Natano]

On Fri, May 27, 2016 at 10:15:39PM +0200, Jeremie Courreges-Anglas wrote:
> Mathieu -  writes:
> 
> > Hello everyone,
> >
> > While playing a bit with ptrace to do some debugging I stumbled upon
> > something that looks like a bug.
> > While trying to write to the ptrace'd process using PT_IO in combinaison
> > with PIOD_WRITE_D I kept getting EFAULTs.
> > PIOD_READ_D would work fine on the same address though, and even more
> > weirdly PIOD_WRITE_I would also work fine on the same address.
> > Even more strange, PT_WRITE_D works fine too on the same address.
> > So in effect, only PT_IO + PIOD_WRITE_D would EFAULT on this address.
> >
> > If this is the expected behavior (I really doubt it), then the man page
> > is wrong because it states clearly that *_I and *_D are equivalent.
> >
> > Digging a bit on the implementation I traced it back to rev 1.33 of
> > sys_process.c.
> > The old implementation used procfs_domem to do the uvm_io call, and
> > based the decision as to use UVM_IO_FIXPROT or not on the uio.uio_rw
> > field (UIO_WRITE meant FIXPROT vs UIO_READ).
> > However the new implementation, in process_domem takes a third
> > parameter, req, the ptrace request and would use UVM_IO_FIXPROT only in
> > the PT_WRITE_I case (rings any bell?).
> > That's why PT_WRITE_D will EFAULT in any case.
> > Oh and PT_WRITE_I and PT_WRITE_D work because they both use PT_WRITE_I
> > as the req parameter :
> >  process_domem(p, t, , write ? PT_WRITE_I : 
> >
> > So I came up with the following diff (kind of the big hammer approach),
> > which gets rid of the req parameters and base the UVM_IO_FIXPROT
> > decision on the uio.uio_rw field as the previous code (10 years ago!)
> > was doing.
> 
> This looks correct to me.  ok / can I get another review?
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 

The diff reads fine to me, however it is incomplete. There are some
callers of process_domem() in arch/. They will need to be changed too.
req seems to be in sync with uio_rw in all the cases, so just removing
the last argument should do it.

natano

Re: replace bpf open loops

[Martin Natano]

On Fri, May 27, 2016 at 10:03:37PM +0200, Jeremie Courreges-Anglas wrote:
> Martin Natano <nat...@natano.net> writes:
> 
> > I think it's time to get rid of all the bpf open() loops in base.
> > dhclient and libpcap do a plain open("/dev/bpf0", ...) since a couple of
> > weeks now and the upgrade issue (/dev/bpf vs. /dev/bpf0) has been fixed.
> > I didn't hear any other complaints in the meantime.
> >
> > Ok? Too soon?
> 
> Looks fine, and I don't see any reason to delay this.  Did you have
> a particular issue in mind?

No.

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

Re: use ${CC} -E rather than ${CPP}

[Martin Natano]

On Thu, May 26, 2016 at 02:14:29PM -0600, Todd C. Miller wrote:
> On Thu, 26 May 2016 22:07:14 +0200, Martin Natano wrote:
> 
> > > diff -u -p -u -r1.11 Makefile
> > > --- share/locale/ctype/Makefile   20 Mar 2016 15:45:40 -  1.11
> > > +++ share/locale/ctype/Makefile   23 Mar 2016 20:16:56 -
> > > @@ -9,8 +9,7 @@ NOMAN=# defined
> > >  all: UTF-8.out
> > >  
> > >  UTF-8.out: en_US.UTF-8.src
> > > - ${CPP} < ${.CURDIR}/en_US.UTF-8.src | \
> > > - sed -e '/^#/d' | mklocale -o ${.TARGET}
> > > + ${CC} -E -P - < ${.CURDIR}/en_US.UTF-8.src | mklocale -o ${.TARGET}
> > 
> > I think cpp can be dropped here. As far as I can tell it is only used to
> > remove the comments, which mklocale can do by itself. The output stays
> > the same with just this:
> > 
> > mklocale -o ${.TARGET} ${.CURDIR}/en_US.UTF-8.src
> 
> I don't see anything in mklocale that handles C-style comments.
> We can check the output:

mklocale.1 says "C-style comments are used to place comments in the
file."

> 
> $ cc -E -P - < en_US.UTF-8.src | mklocale -o foo
> $ mklocale -o bar en_US.UTF-8.src
> $ cmp foo bar
> foo bar differ: char 3132, line 6

Yes, the difference is due to cpp vs. cc -E. Try to diff against the
output generated with cpp. Then the files are the same.

$ cpp < en_US.UTF-8.src | mklocale -o foo
$ cc -E -P - < en_US.UTF-8.src | mklocale -o foo2
$ mklocale -o foo3 en_US.UTF-8.src
$ cmp foo foo2
foo foo2 differ: char 3132, line 4
$ cmp foo foo3
$


> 
> I think that file does need preprocessing.
> 
>  - todd

Re: use ${CC} -E rather than ${CPP}

[Martin Natano]

See one comment inline.

On Thu, May 26, 2016 at 09:59:24AM -0600, Todd C. Miller wrote:
> Since /usr/bin/cpp still specified -traditional, use ${CC} -E
> instead.  I've had this rotting in my tree for years now.
> 
>  - todd
> 
> Index: lib/libcurses/Makefile
> ===
> RCS file: /cvs/src/lib/libcurses/Makefile,v
> retrieving revision 1.67
> diff -u -p -u -r1.67 Makefile
> --- lib/libcurses/Makefile30 Mar 2016 06:38:42 -  1.67
> +++ lib/libcurses/Makefile26 May 2016 15:38:57 -
> @@ -118,7 +118,7 @@ fallback.c: ${.CURDIR}/tinfo/MKfallback.
>   sh ${.CURDIR}/tinfo/MKfallback.sh $(FALLBACK_LIST) > ${.TARGET}
>  
>  lib_gen.c: ${.CURDIR}/base/MKlib_gen.sh
> - sh ${.CURDIR}/base/MKlib_gen.sh "${CPP} -I${.CURDIR}" \
> + sh ${.CURDIR}/base/MKlib_gen.sh "${CC} -E -I${.CURDIR}" \
>   "${AWK}" generated < ${.CURDIR}/curses.h > lib_gen.c
>  
>  init_keytry.h: make_keys keys.list
> @@ -135,7 +135,7 @@ make_hash: ${.CURDIR}/tinfo/comp_hash.c 
>  
>  expanded.c: ${.CURDIR}/term.h ${.CURDIR}/curses.priv.h \
>   ${.CURDIR}/ncurses_cfg.h ${.CURDIR}/tty/MKexpanded.sh
> - sh ${.CURDIR}/tty/MKexpanded.sh "${CPP}" ${CPPFLAGS} > ${.TARGET}
> + sh ${.CURDIR}/tty/MKexpanded.sh "${CC} -E" ${CPPFLAGS} > ${.TARGET}
>  
>  comp_captab.c: make_hash
>   sh ${.CURDIR}/tinfo/MKcaptab.sh ${AWK} 1 \
> Index: share/locale/ctype/Makefile
> ===
> RCS file: /cvs/src/share/locale/ctype/Makefile,v
> retrieving revision 1.11
> diff -u -p -u -r1.11 Makefile
> --- share/locale/ctype/Makefile   20 Mar 2016 15:45:40 -  1.11
> +++ share/locale/ctype/Makefile   23 Mar 2016 20:16:56 -
> @@ -9,8 +9,7 @@ NOMAN=# defined
>  all: UTF-8.out
>  
>  UTF-8.out: en_US.UTF-8.src
> - ${CPP} < ${.CURDIR}/en_US.UTF-8.src | \
> - sed -e '/^#/d' | mklocale -o ${.TARGET}
> + ${CC} -E -P - < ${.CURDIR}/en_US.UTF-8.src | mklocale -o ${.TARGET}

I think cpp can be dropped here. As far as I can tell it is only used to
remove the comments, which mklocale can do by itself. The output stays
the same with just this:

mklocale -o ${.TARGET} ${.CURDIR}/en_US.UTF-8.src


>  
>  CLEANFILES+= UTF-8.out
>  
> Index: usr.bin/which/Makefile
> ===
> RCS file: /cvs/src/usr.bin/which/Makefile,v
> retrieving revision 1.8
> diff -u -p -u -r1.8 Makefile
> --- usr.bin/which/Makefile15 Apr 2013 16:34:19 -  1.8
> +++ usr.bin/which/Makefile3 Dec 2013 23:48:18 -
> @@ -7,7 +7,7 @@ LINKS=${BINDIR}/which ${BINDIR}/whereis
>  check_path_in_man:
>   @echo "Checking path expansion in whereis.1"; \
>   stdpath=`printf '#include \n_PATH_STDPATH\n' | \
> - ${CPP} ${CPPFLAGS} - | \
> + ${CC} -E -P ${CPPFLAGS} - | \
>   sed -n 's/^[]*"\(.*\)".*/.D1 \1/p'` ; \
>   fgrep -xq "$$stdpath" ${.CURDIR}/whereis.1 && { touch $@; exit 0; }; \
>   echo "Update the expansion of _PATH_STDPATH in ${.CURDIR}/whereis.1"; \
>