Re: sndiod: Remove ambiguous use of 'last' in man page

2021-12-18 Thread Alexandre Ratchov
On Sat, Dec 18, 2021 at 10:27:34AM +, Jason McIntyre wrote:
> On Sat, Dec 18, 2021 at 10:51:28AM +0100, Richard Ulmer wrote:
> > Hi,
> > after reading about using USB audio interfaces in
> > https://www.openbsd.org/faq/faq13.html I looked up what -F and -f mean
> > in sndiod(8) but was briefly confused by the word 'last'. I commonly
> > understand 'last' as either 'previous' or 'at the end of the list' and
> > in this case the latter popped into my head first, which is the wrong
> > meaning in this case (if I'm not misunderstanding things).
> > 
> 
> hi.
> 
> i agree that "previous" is probably less ambiguous, so i'm ok with that
> change.
> 
> > Maybe the attached patch could prevent this confusion. I also changed
> > 'options' to singular, because it seems to me like it refers to the
> > (single) previous -f/-F option.
> > 
> 
> this one really depends on how the options are thought of, so either
> version is ok really. but with your proposed word change of "previous",
> singulAr does read better.
> 
> so, i'm fine with this diff and happy to commit. i'll let it sit a
> little in case anyone has an issue (i.e. we've both misunderstood the
> text!)
> 

thanks, both changes makes the man page better explain what the
code does

ok ratchov



Re: : provide ElfW() macro

2021-12-18 Thread Theo de Raadt
Klemens Nanni  wrote:

> On Sat, Dec 18, 2021 at 04:37:17PM +0300, Andrew Krasavin wrote:
> > On Fri, Dec 17, 2021 at 01:04:35PM +, Klemens Nanni wrote:
> > > An upcoming port uses ElfW().  I first patched the port, but looking
> > > around shows that both NetBSD and FreeBSD adopted the macro:
> > > 
> > > https://mail-index.netbsd.org/source-changes-hg/2018/07/31/msg027523.html
> > > +#defineElfW(x) CONCAT(Elf,CONCAT(ELFSIZE,CONCAT(_,x)))
> > > 
> > > https://cgit.freebsd.org/src/commit/sys/sys/elf_generic.h?id=34e7e4b6a059eee5e4e3e34de5b9d5f0d6e589f9
> > > +/* Define ElfW for compatibility with Linux, prefer __ElfN() in FreeBSD 
> > > code */
> > > +#define  ElfW(x) __ElfN(x)
> > > 
> > > So I suggest providing it as well.
> > > 
> > > I have yet to put this through a release build on amd64 and/or sparc64
> > > to check if there is any fallout, but I don't suspect there will be.
> > > 
> > > Feedback? Objections? OK (given the build succeeds)?
> > > 
> > > 
> > > Index: sys/sys/exec_elf.h
> > > ===
> > > RCS file: /cvs/src/sys/sys/exec_elf.h,v
> > > retrieving revision 1.93
> > > diff -u -p -r1.93 exec_elf.h
> > > --- sys/sys/exec_elf.h7 Dec 2021 22:17:03 -   1.93
> > > +++ sys/sys/exec_elf.h17 Dec 2021 12:36:39 -
> > > @@ -725,6 +725,7 @@ struct elf_args {
> > > #define CONCAT(x,y)   __CONCAT(x,y)
> > > #define ELFNAME(x)CONCAT(elf,CONCAT(ELFSIZE,CONCAT(_,x)))
> > > #define ELFDEFNNAME(x)CONCAT(ELF,CONCAT(ELFSIZE,CONCAT(_,x)))
> > > +#define ElfW(x)  CONCAT(Elf,CONCAT(ELFSIZE,CONCAT(_,x)))
> > > #endif
> > > 
> > > #if defined(ELFSIZE) && (ELFSIZE == 32)
> > > 
> > 
> > The code that requires this macro in sys/sys/exec_elf.h was
> > eventually removed from the upcoming port (devel/abseil-cpp).
> > 
> > I have no opinion on what you should do with this patch now, but
> > I feel it is necessary to report that this change is no longer
> > needed for my port.
> 
> Even better!
> 
> Unless there are objections, I'll make sure that this doesn't break
> anything and then commit it, so that future ports that do require it
> will build -- there is no downside to providing it, imho.

Why would you commit an unneccessary line to a protected namespace
which doesn't need it?



Re: : provide ElfW() macro

2021-12-18 Thread Klemens Nanni
On Sat, Dec 18, 2021 at 04:37:17PM +0300, Andrew Krasavin wrote:
> On Fri, Dec 17, 2021 at 01:04:35PM +, Klemens Nanni wrote:
> > An upcoming port uses ElfW().  I first patched the port, but looking
> > around shows that both NetBSD and FreeBSD adopted the macro:
> > 
> > https://mail-index.netbsd.org/source-changes-hg/2018/07/31/msg027523.html
> > +#defineElfW(x) CONCAT(Elf,CONCAT(ELFSIZE,CONCAT(_,x)))
> > 
> > https://cgit.freebsd.org/src/commit/sys/sys/elf_generic.h?id=34e7e4b6a059eee5e4e3e34de5b9d5f0d6e589f9
> > +/* Define ElfW for compatibility with Linux, prefer __ElfN() in FreeBSD 
> > code */
> > +#defineElfW(x) __ElfN(x)
> > 
> > So I suggest providing it as well.
> > 
> > I have yet to put this through a release build on amd64 and/or sparc64
> > to check if there is any fallout, but I don't suspect there will be.
> > 
> > Feedback? Objections? OK (given the build succeeds)?
> > 
> > 
> > Index: sys/sys/exec_elf.h
> > ===
> > RCS file: /cvs/src/sys/sys/exec_elf.h,v
> > retrieving revision 1.93
> > diff -u -p -r1.93 exec_elf.h
> > --- sys/sys/exec_elf.h  7 Dec 2021 22:17:03 -   1.93
> > +++ sys/sys/exec_elf.h  17 Dec 2021 12:36:39 -
> > @@ -725,6 +725,7 @@ struct elf_args {
> > #define CONCAT(x,y) __CONCAT(x,y)
> > #define ELFNAME(x)  CONCAT(elf,CONCAT(ELFSIZE,CONCAT(_,x)))
> > #define ELFDEFNNAME(x)  CONCAT(ELF,CONCAT(ELFSIZE,CONCAT(_,x)))
> > +#define ElfW(x)CONCAT(Elf,CONCAT(ELFSIZE,CONCAT(_,x)))
> > #endif
> > 
> > #if defined(ELFSIZE) && (ELFSIZE == 32)
> > 
> 
> The code that requires this macro in sys/sys/exec_elf.h was
> eventually removed from the upcoming port (devel/abseil-cpp).
> 
> I have no opinion on what you should do with this patch now, but
> I feel it is necessary to report that this change is no longer
> needed for my port.

Even better!

Unless there are objections, I'll make sure that this doesn't break
anything and then commit it, so that future ports that do require it
will build -- there is no downside to providing it, imho.

> 
> -- 
> Wbr, Andrew Krasavin
> 



Re: : provide ElfW() macro

2021-12-18 Thread Andrew Krasavin

On Fri, Dec 17, 2021 at 01:04:35PM +, Klemens Nanni wrote:

An upcoming port uses ElfW().  I first patched the port, but looking
around shows that both NetBSD and FreeBSD adopted the macro:

https://mail-index.netbsd.org/source-changes-hg/2018/07/31/msg027523.html
+#defineElfW(x) CONCAT(Elf,CONCAT(ELFSIZE,CONCAT(_,x)))

https://cgit.freebsd.org/src/commit/sys/sys/elf_generic.h?id=34e7e4b6a059eee5e4e3e34de5b9d5f0d6e589f9
+/* Define ElfW for compatibility with Linux, prefer __ElfN() in FreeBSD code */
+#defineElfW(x) __ElfN(x)

So I suggest providing it as well.

I have yet to put this through a release build on amd64 and/or sparc64
to check if there is any fallout, but I don't suspect there will be.

Feedback? Objections? OK (given the build succeeds)?


Index: sys/sys/exec_elf.h
===
RCS file: /cvs/src/sys/sys/exec_elf.h,v
retrieving revision 1.93
diff -u -p -r1.93 exec_elf.h
--- sys/sys/exec_elf.h  7 Dec 2021 22:17:03 -   1.93
+++ sys/sys/exec_elf.h  17 Dec 2021 12:36:39 -
@@ -725,6 +725,7 @@ struct elf_args {
#define CONCAT(x,y) __CONCAT(x,y)
#define ELFNAME(x)  CONCAT(elf,CONCAT(ELFSIZE,CONCAT(_,x)))
#define ELFDEFNNAME(x)  CONCAT(ELF,CONCAT(ELFSIZE,CONCAT(_,x)))
+#define ElfW(x)CONCAT(Elf,CONCAT(ELFSIZE,CONCAT(_,x)))
#endif

#if defined(ELFSIZE) && (ELFSIZE == 32)



The code that requires this macro in sys/sys/exec_elf.h was
eventually removed from the upcoming port (devel/abseil-cpp).

I have no opinion on what you should do with this patch now, but
I feel it is necessary to report that this change is no longer
needed for my port.

--
Wbr, Andrew Krasavin



Re: sndiod: Remove ambiguous use of 'last' in man page

2021-12-18 Thread Jason McIntyre
On Sat, Dec 18, 2021 at 10:51:28AM +0100, Richard Ulmer wrote:
> Hi,
> after reading about using USB audio interfaces in
> https://www.openbsd.org/faq/faq13.html I looked up what -F and -f mean
> in sndiod(8) but was briefly confused by the word 'last'. I commonly
> understand 'last' as either 'previous' or 'at the end of the list' and
> in this case the latter popped into my head first, which is the wrong
> meaning in this case (if I'm not misunderstanding things).
> 

hi.

i agree that "previous" is probably less ambiguous, so i'm ok with that
change.

> Maybe the attached patch could prevent this confusion. I also changed
> 'options' to singular, because it seems to me like it refers to the
> (single) previous -f/-F option.
> 

this one really depends on how the options are thought of, so either
version is ok really. but with your proposed word change of "previous",
singulAr does read better.

so, i'm fine with this diff and happy to commit. i'll let it sit a
little in case anyone has an issue (i.e. we've both misunderstood the
text!)

jmc

> - Richard Ulmer
> 
> 
> Index: sndiod.8
> ===
> RCS file: /cvs/src/usr.bin/sndiod/sndiod.8,v
> retrieving revision 1.11
> diff -u -p -u -r1.11 sndiod.8
> --- sndiod.816 Jul 2021 15:05:58 -  1.11
> +++ sndiod.818 Dec 2021 09:42:07 -
> @@ -186,11 +186,11 @@ Examples:
>  .Va u8 , s16le , s24le3 , s24le4lsb .
>  .It Fl F Ar device
>  Specify an alternate device to use.
> -If it doesn't work, the one given with the last
> +If it doesn't work, the one given with the previous
>  .Fl f
>  or
>  .Fl F
> -options will be used.
> +option will be used.
>  For instance, specifying a USB device following a
>  PCI device allows
>  .Nm
> 



Re: fix ping(8) and traceroute(8) source selection

2021-12-18 Thread Denis Fondras
Le Sat, Dec 18, 2021 at 10:02:32AM +0100, Florian Obser a écrit :
> On 2021-12-17 22:12 +01, Denis Fondras  wrote:
> > Here is an attempt to fix ping(8) and traceroute(8) source selection.
> >
> > Currently these tools do not obey route sourceaddr set by the operator. This
> > leads to frustration at best and erroneous diagnosis at worse on multi-homed
> > systems.
> 
> I did not closely follow route(8)'s sourceaddr feature. Is this only an
> issue with IPv4 or would ping6 / traceroute6 need a similar fix (which
> is going to be difficult).
> 

IPv6 is immune because it is the responsability of the caller to set a valid
source address (unless it is DAD packet).



Re: fix ping(8) and traceroute(8) source selection

2021-12-18 Thread Florian Obser
On 2021-12-17 22:12 +01, Denis Fondras  wrote:
> Here is an attempt to fix ping(8) and traceroute(8) source selection.
>
> Currently these tools do not obey route sourceaddr set by the operator. This
> leads to frustration at best and erroneous diagnosis at worse on multi-homed
> systems.

I did not closely follow route(8)'s sourceaddr feature. Is this only an
issue with IPv4 or would ping6 / traceroute6 need a similar fix (which
is going to be difficult).

>
> The "real" fix would be to rework source selection in the kernel stack but 
> this
> is a huge work which not happen overnight nor in the coming days.
>
> In the mean time, I propose the following diff.
>
> I removed -R (route recording) in ping(8) because it is not compatible with
> sending a full IP header to the rip_output(). It should not impact anyone as 
> RR
> is most of the time ignored by routers.

I was toying with removing -R when I was unifying ping and ping6, but
then I found a system that did allow it. IIRC one needs to disable pf on
OpenBSD, so the utility is indeed very limited. Maybe it's time for it
to go.

>
> Denis
>
> Index: sbin/ping/ping.c
> ===
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.245
> diff -u -p -r1.245 ping.c
> --- sbin/ping/ping.c  12 Jul 2021 15:09:19 -  1.245
> +++ sbin/ping/ping.c  17 Dec 2021 20:27:31 -
> @@ -143,16 +143,14 @@ int options;
>  #define  F_HOSTNAME  0x0004
>  #define  F_PINGFILLED0x0008
>  #define  F_QUIET 0x0010
> -#define  F_RROUTE0x0020
> -#define  F_SO_DEBUG  0x0040
> -#define  F_SHOWCHAR  0x0080
> -#define  F_VERBOSE   0x0100
> +#define  F_SO_DEBUG  0x0020
> +#define  F_SHOWCHAR  0x0040
> +#define  F_VERBOSE   0x0080

no need for this churn

>  /*   0x0200 */

you could just add a comment like this ^

> -#define  F_HDRINCL   0x0400
> -#define  F_TTL   0x0800
> -#define  F_TOS   0x1000
> -#define  F_AUD_RECV  0x2000
> -#define  F_AUD_MISS  0x4000
> +#define  F_TTL   0x0100
> +#define  F_TOS   0x0200
> +#define  F_AUD_RECV  0x0400
> +#define  F_AUD_MISS  0x0800

I'm with deraadt@ that the kernel implementation should be finished.

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