Re: CVS commit: src/sys/arch/i386/stand/lib

[Rin Okuyama]

Hi,

On 2024/09/19 3:33, Andrius V wrote:

On Wed, Sep 18, 2024 at 3:44 AM Rin Okuyama  wrote:


Module Name:src
Committed By:   rin
Date:   Wed Sep 18 00:44:03 UTC 2024

Modified Files:
 src/sys/arch/i386/stand/lib: libi386.h

Log Message:
i386/stand: Remove XMS leftover from libi386.h, NFC

PR port-i386/58624


To generate a diff of this commit:
cvs rdiff -u -r1.53 -r1.54 src/sys/arch/i386/stand/lib/libi386.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



Thanks, not sure how it slipped (I have definitely seen the code,
either forgot to copy to cvs directory or just forgot about it).



It can be... Your work was otherwise perfect. Thank you very much for
cleaning up things including tiny comments!

rin

Re: CVS commit: src/sys/arch/i386/stand/lib

[Andrius V]

On Wed, Sep 18, 2024 at 3:44 AM Rin Okuyama  wrote:
>
> Module Name:src
> Committed By:   rin
> Date:   Wed Sep 18 00:44:03 UTC 2024
>
> Modified Files:
> src/sys/arch/i386/stand/lib: libi386.h
>
> Log Message:
> i386/stand: Remove XMS leftover from libi386.h, NFC
>
> PR port-i386/58624
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.53 -r1.54 src/sys/arch/i386/stand/lib/libi386.h
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>

Thanks, not sure how it slipped (I have definitely seen the code,
either forgot to copy to cvs directory or just forgot about it).

Re: CVS commit: src/sys/arch/mac68k/dev

[Rin Okuyama]

Thank you very much for kind & rapid response!!

rin

On 2024/09/18 10:34, Nathanial Sloss wrote:

Module Name:src
Committed By:   nat
Date:   Wed Sep 18 01:34:08 UTC 2024

Modified Files:
src/sys/arch/mac68k/dev: adb.c

Log Message:
The delay is only required for machines with built-in floppy drives.

I doubt the vibration from an external drive on the Duos would affect adb
before it attaches.


To generate a diff of this commit:
cvs rdiff -u -r1.61 -r1.62 src/sys/arch/mac68k/dev/adb.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.


Modified files:

Index: src/sys/arch/mac68k/dev/adb.c
diff -u src/sys/arch/mac68k/dev/adb.c:1.61 src/sys/arch/mac68k/dev/adb.c:1.62
--- src/sys/arch/mac68k/dev/adb.c:1.61  Wed Sep 18 01:29:02 2024
+++ src/sys/arch/mac68k/dev/adb.c   Wed Sep 18 01:34:08 2024
@@ -1,4 +1,4 @@
-/* $NetBSD: adb.c,v 1.61 2024/09/18 01:29:02 nat Exp $ */
+/* $NetBSD: adb.c,v 1.62 2024/09/18 01:34:08 nat Exp $ */
  
  /*

   * Copyright (C) 1994 Bradley A. Grantham
@@ -26,7 +26,7 @@
   */
  
  #include 

-__KERNEL_RCSID(0, "$NetBSD: adb.c,v 1.61 2024/09/18 01:29:02 nat Exp $");
+__KERNEL_RCSID(0, "$NetBSD: adb.c,v 1.62 2024/09/18 01:34:08 nat Exp $");
  
  #include "opt_adb.h"
  
@@ -104,12 +104,6 @@ adbattach(device_t parent, device_t self

case MACH_MACPB180:
case MACH_MACPB180C:
case MACH_MACPB150:
-   case MACH_MACPB210:
-   case MACH_MACPB230:
-   case MACH_MACPB250:
-   case MACH_MACPB270:
-   case MACH_MACPB280:
-   case MACH_MACPB280C:
/* Reqired on machines with in-built trackballs. */
printf("...  Waiting 5 seconds for adb devices to "
 "settle.");

Re: CVS commit: src/sys/arch/mac68k/dev

[Rin Okuyama]

Hi,

Can you please localize this quirk only for the affected machines?
It seems too much to me, to have 5 sec delay for irrelevant machines.

Thanks,
rin

On 2024/09/15 5:56, Nathanial Sloss wrote:

Module Name:src
Committed By:   nat
Date:   Sat Sep 14 20:56:51 UTC 2024

Modified Files:
src/sys/arch/mac68k/dev: adb.c

Log Message:
Allow a few seconds for adb devices to settle.

The spin up of the floppy drive motor would cause a crash on my PowerBook
when adb was attached.


To generate a diff of this commit:
cvs rdiff -u -r1.59 -r1.60 src/sys/arch/mac68k/dev/adb.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.


Modified files:

Index: src/sys/arch/mac68k/dev/adb.c
diff -u src/sys/arch/mac68k/dev/adb.c:1.59 src/sys/arch/mac68k/dev/adb.c:1.60
--- src/sys/arch/mac68k/dev/adb.c:1.59  Sat Aug  7 16:18:57 2021
+++ src/sys/arch/mac68k/dev/adb.c   Sat Sep 14 20:56:50 2024
@@ -1,4 +1,4 @@
-/* $NetBSD: adb.c,v 1.59 2021/08/07 16:18:57 thorpej Exp $ */
+/* $NetBSD: adb.c,v 1.60 2024/09/14 20:56:50 nat Exp $ */
  
  /*

   * Copyright (C) 1994 Bradley A. Grantham
@@ -26,7 +26,7 @@
   */
  
  #include 

-__KERNEL_RCSID(0, "$NetBSD: adb.c,v 1.59 2021/08/07 16:18:57 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: adb.c,v 1.60 2024/09/14 20:56:50 nat Exp $");
  
  #include "opt_adb.h"
  
@@ -94,6 +94,9 @@ static void

  adbattach(device_t parent, device_t self, void *aux)
  {
  
+	printf("...  Waiting 5 seconds for adb devices to settle.");

+   kpause("adb-slp", false, mstohz(5000), NULL);
+
adb_softintr_cookie = softint_establish(SOFTINT_SERIAL,
(void (*)(void *))adb_soft_intr, NULL);
printf("\n");

Re: CVS commit: src/sys/arch/x86/x86

[Valery Ushakov]

On Wed, Sep 11, 2024 at 05:17:45 +, matthew green wrote:

> Module Name:  src
> Committed By: mrg
> Date: Wed Sep 11 05:17:45 UTC 2024
> 
> Modified Files:
>   src/sys/arch/x86/x86: intr.c
> 
> Log Message:
> apply some more diagnostic checks for x86 interrupts

How does this mix with KDTRACE_HOOKS?  To paraphrase:

#define IS_I8254_CLOCKINTR \
(handler == __FPTRCAST(int (*)(void *), i8254_clockintr))

#ifdef KDTRACE_HOOKS
if (!IS_I8254_CLOCKINTR) {
ih->ih_fun = intr_kdtrace_wrapper;
ih->ih_arg = ih;
}
#endif
#ifdef MULTIPROCESSOR
if (!mpsafe) {
KASSERT(!IS_I8254_CLOCKINTR);
ih->ih_fun = intr_biglock_wrapper;
ih->ih_arg = ih;
}
#ifdef DIAGNOSTIC   /* wrap all interrupts */
else if (!IS_I8254_CLOCKINTR) {
ih->ih_fun = intr_wrapper;
ih->ih_arg = ih;
}
#endif
#endif /* MULTIPROCESSOR */

and MULTIPROCESSOR case overwrites whatever KDTRACE_HOOKS did, doesn't
it?

-uwe

Re: CVS commit: src/lib/libc

[Taylor R Campbell]

> Date: Sat, 14 Sep 2024 22:26:37 - (UTC)
> From: chris...@astron.com (Christos Zoulas)
> 
> In article ,
> Thomas Klausner   wrote:
> >On Wed, Sep 11, 2024 at 09:50:35AM -0400, Christos Zoulas wrote:
> >> POSIX.1-2024 removes asctime_r and ctime_r and does not let
> >> libraries define them, so remove them except when needed to
> >> conform to earlier POSIX.  These functions are dangerous as they
> >> can overrun user buffers.  If you still need them, add
> >> -DSUPPORT_POSIX2008 to CFLAGS.
> >
> >Hm, that sounds like we should hide asctime_r and ctime_r?
> 
> I think that it will break stuff in pkgsrc... We could, I guess.

We have to continue defining the symbols in libc.  We can put the
declarations in time.h under

#if (_POSIX_C_SOURCE - 0 < 202405L) || defined(_NETBSD_SOURCE)
...
#endif

in addition to whatever conditions are already there.

The _POSIX_C_SOURCE part is mandatory for POSIX.1-2024 compliance.

The _NETBSD_SOURCE part is up to us and we could choose to remove it
later (or invent a date system for _NETBSD_SOURCE like
_POSIX_C_SOURCE), or, rather, replace it by __LIBC12_SOURCE__ so the
libc definitions still work.

Re: CVS commit: src/lib/libc

[Greg Troxel]

chris...@astron.com (Christos Zoulas) writes:

>>> POSIX.1-2024 removes asctime_r and ctime_r and does not let
>>> libraries define them, so remove them except when needed to
>>> conform to earlier POSIX.  These functions are dangerous as they
>>> can overrun user buffers.  If you still need them, add
>>> -DSUPPORT_POSIX2008 to CFLAGS.
>>
>>Hm, that sounds like we should hide asctime_r and ctime_r?
>>
>
> I think that it will break stuff in pkgsrc... We could, I guess.

Visisbility define use is a nice theory but in practice it's a mess.

Probably, before hiding, someone(tm) should build a system with them
hidden and do a bulk build.

More seriously, NetBSD having breaking changes generally does not go
well, unless Linux has had the change for a year, so that upstreams have
had time to adapt.

Re: CVS commit: src/lib/libc

[Christos Zoulas]

In article ,
Thomas Klausner   wrote:
>On Wed, Sep 11, 2024 at 09:50:35AM -0400, Christos Zoulas wrote:
>> Module Name: src
>> Committed By:christos
>> Date:Wed Sep 11 13:50:35 UTC 2024
>> 
>> Modified Files:
>>  src/lib/libc/compat/time: compat_localtime.c
>>  src/lib/libc/time: CONTRIBUTING Makefile Makefile.inc NEWS asctime.c
>>  localtime.c private.h theory.html tz-art.html tz-link.html tzfile.5
>>  tzselect.ksh version zdump.c zic.8 zic.c
>> 
>> Log Message:
>> Merge tzcode-2024b
>> 
>> Release 2024b - 2024-09-04 12:27:47 -0700
>> 
>>   Changes to code
>> 
>> localtime.c now always uses a TZif file's time type 0 to handle
>> timestamps before the file's first transition.  Formerly,
>> localtime.c sometimes inferred a different time type, in order to
>> handle problematic data generated by zic 2018e or earlier.  As it
>> is now safe to assume more recent versions of zic, there is no
>> longer a pressing need to fail to conform RFC 8536 section 3.2,
>> which requires using time type 0 in this situation.  This change
>> does not affect behavior when reading TZif files generated by zic
>> 2018f and later.
>> 
>> POSIX.1-2024 removes asctime_r and ctime_r and does not let
>> libraries define them, so remove them except when needed to
>> conform to earlier POSIX.  These functions are dangerous as they
>> can overrun user buffers.  If you still need them, add
>> -DSUPPORT_POSIX2008 to CFLAGS.
>
>Hm, that sounds like we should hide asctime_r and ctime_r?
>

I think that it will break stuff in pkgsrc... We could, I guess.

christos

Re: CVS commit: src/sys/miscfs/procfs

[Paul Goyette]

This was also supposed to include in the log message:

Include mqueue-based entries only if built with MQUEUE option.


On Sat, 14 Sep 2024, Paul Goyette wrote:


Module Name:src
Committed By:   pgoyette
Date:   Sat Sep 14 01:37:42 UTC 2024

Modified Files:
src/sys/miscfs/procfs: procfs_linux.c procfs_vfsops.c

Log Message:
Define dependencies based on build options.


To generate a diff of this commit:
cvs rdiff -u -r1.89 -r1.90 src/sys/miscfs/procfs/procfs_linux.c
cvs rdiff -u -r1.119 -r1.120 src/sys/miscfs/procfs/procfs_vfsops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.


!DSPAM:66e4e8e8182295181515653!




+-+--+--+
| Paul Goyette (.sig) | PGP Key fingerprint: | E-mail addresses:|
| (Retired)   | 1B11 1849 721C 56C8 F63A | p...@whooppee.com|
| Software Developer  | 6E2E 05FD 15CE 9F2D 5102 | pgoye...@netbsd.org  |
| & Network Engineer  |  | pgoyett...@gmail.com |
+-+--+--+

Re: CVS commit: src/sys/sys

[Ryo ONODERA]

Hi,

Christos Zoulas  writes:

> And committed.

Thank you very much for your quick fix.
It works fine for me now.

> christos
>
>

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/lib/libc

[Thomas Klausner]

On Wed, Sep 11, 2024 at 09:50:35AM -0400, Christos Zoulas wrote:
> Module Name:  src
> Committed By: christos
> Date: Wed Sep 11 13:50:35 UTC 2024
> 
> Modified Files:
>   src/lib/libc/compat/time: compat_localtime.c
>   src/lib/libc/time: CONTRIBUTING Makefile Makefile.inc NEWS asctime.c
>   localtime.c private.h theory.html tz-art.html tz-link.html tzfile.5
>   tzselect.ksh version zdump.c zic.8 zic.c
> 
> Log Message:
> Merge tzcode-2024b
> 
> Release 2024b - 2024-09-04 12:27:47 -0700
> 
>   Changes to code
> 
> localtime.c now always uses a TZif file's time type 0 to handle
> timestamps before the file's first transition.  Formerly,
> localtime.c sometimes inferred a different time type, in order to
> handle problematic data generated by zic 2018e or earlier.  As it
> is now safe to assume more recent versions of zic, there is no
> longer a pressing need to fail to conform RFC 8536 section 3.2,
> which requires using time type 0 in this situation.  This change
> does not affect behavior when reading TZif files generated by zic
> 2018f and later.
> 
> POSIX.1-2024 removes asctime_r and ctime_r and does not let
> libraries define them, so remove them except when needed to
> conform to earlier POSIX.  These functions are dangerous as they
> can overrun user buffers.  If you still need them, add
> -DSUPPORT_POSIX2008 to CFLAGS.

Hm, that sounds like we should hide asctime_r and ctime_r?
 Thomas

Re: CVS commit: src/sys/sys

[Christos Zoulas]

And committed.

christos

Re: CVS commit: src/sys/sys

[Christos Zoulas]

On 2024-09-11 11:18 am, Taylor R Campbell wrote:

Date: Thu, 12 Sep 2024 00:05:24 +0900
From: Ryo ONODERA 

"Taylor R Campbell"  writes:

> sys/endian.h: Hide le32enc/be32enc/... under _NETBSD_SOURCE.
>
> These are non-standard extensions, so they should not be exposed by,
> e.g., _XOPEN_SOURCE=700.
>
> PR standards/57807: #include  spuriously defines
> le32enc/be32enc/... under _XOPEN_SOURCE=700

After this change, -current build on -current is broken,
because tools/libctf defines _NETBSD_SOURCE and le32dec and so on
cause conflicts.

I think HAVE_NBTOOLS_CONFIG_H part in 
external/cddl/osnet/sys/sys/types.h

may be problematic. However it is not clear how to fix properly.

Could you take a look at this problem?


christos was looking at this yesterday -- christos, did you make
progress after our discussion?


P.S.
I feel that HAVE_NBTOOLS_CONFIG_H should be HAVE_NBTOOL_CONFIG_H.


Yes, except I think that whole stanza under HAVE_NBTOOL[S]_CONFIG_H is
wrong and should be replaced by an _unconditional_

#include_next 
#include_next 

without any _NETBSD_SOURCE games (which the tools build should never
play; we go out of our way to use _XOPEN_SOURCE=600 in compat_defs.h
so that _NETBSD_SOURCE does not get used everywhere else in the tools
build, in order to keep the tools build clean and portable).


Yes, close to finish the build and commit changes.

--
christos

Re: CVS commit: src/sys/sys

[Taylor R Campbell]

> Date: Thu, 12 Sep 2024 00:05:24 +0900
> From: Ryo ONODERA 
> 
> "Taylor R Campbell"  writes:
> 
> > sys/endian.h: Hide le32enc/be32enc/... under _NETBSD_SOURCE.
> >
> > These are non-standard extensions, so they should not be exposed by,
> > e.g., _XOPEN_SOURCE=700.
> >
> > PR standards/57807: #include  spuriously defines
> > le32enc/be32enc/... under _XOPEN_SOURCE=700
> 
> After this change, -current build on -current is broken,
> because tools/libctf defines _NETBSD_SOURCE and le32dec and so on
> cause conflicts.
> 
> I think HAVE_NBTOOLS_CONFIG_H part in external/cddl/osnet/sys/sys/types.h
> may be problematic. However it is not clear how to fix properly.
> 
> Could you take a look at this problem?

christos was looking at this yesterday -- christos, did you make
progress after our discussion?

> P.S.
> I feel that HAVE_NBTOOLS_CONFIG_H should be HAVE_NBTOOL_CONFIG_H.

Yes, except I think that whole stanza under HAVE_NBTOOL[S]_CONFIG_H is
wrong and should be replaced by an _unconditional_

#include_next 
#include_next 

without any _NETBSD_SOURCE games (which the tools build should never
play; we go out of our way to use _XOPEN_SOURCE=600 in compat_defs.h
so that _NETBSD_SOURCE does not get used everywhere else in the tools
build, in order to keep the tools build clean and portable).

Re: CVS commit: src/sys/sys

[Ryo ONODERA]

Hi,

"Taylor R Campbell"  writes:

> Module Name:  src
> Committed By: riastradh
> Date: Mon Sep  9 18:17:14 UTC 2024
>
> Modified Files:
>   src/sys/sys: endian.h
>
> Log Message:
> sys/endian.h: Hide le32enc/be32enc/... under _NETBSD_SOURCE.
>
> These are non-standard extensions, so they should not be exposed by,
> e.g., _XOPEN_SOURCE=700.
>
> PR standards/57807: #include  spuriously defines
> le32enc/be32enc/... under _XOPEN_SOURCE=700

After this change, -current build on -current is broken,
because tools/libctf defines _NETBSD_SOURCE and le32dec and so on
cause conflicts.

I think HAVE_NBTOOLS_CONFIG_H part in external/cddl/osnet/sys/sys/types.h
may be problematic. However it is not clear how to fix properly.

Could you take a look at this problem?

P.S.
I feel that HAVE_NBTOOLS_CONFIG_H should be HAVE_NBTOOL_CONFIG_H.

Thank you.

> To generate a diff of this commit:
> cvs rdiff -u -r1.33 -r1.34 src/sys/sys/endian.h
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
> Modified files:
>
> Index: src/sys/sys/endian.h
> diff -u src/sys/sys/endian.h:1.33 src/sys/sys/endian.h:1.34
> --- src/sys/sys/endian.h:1.33 Mon Sep  9 15:22:50 2024
> +++ src/sys/sys/endian.h  Mon Sep  9 18:17:14 2024
> @@ -1,4 +1,4 @@
> -/*   $NetBSD: endian.h,v 1.33 2024/09/09 15:22:50 riastradh Exp $*/
> +/*   $NetBSD: endian.h,v 1.34 2024/09/09 18:17:14 riastradh Exp $*/
>  
>  /*
>   * Copyright (c) 1987, 1991, 1993
> @@ -192,6 +192,8 @@ __END_DECLS
>   * to/from an octet stream.
>   */
>  
> +#ifdef _NETBSD_SOURCE
> +
>  #if __GNUC_PREREQ__(2, 95)
>  
>  #define __GEN_ENDIAN_ENC(bits, endian) \
> @@ -337,6 +339,8 @@ le64dec(const void *buf)
>  
>  #endif   /* GCC >= 2.95 */
>  
> +#endif   /* _NETBSD_SOURCE */
> +
>  #endif /* !_LOCORE */
>  #endif /* _XOPEN_SOURCE || _NETBSD_SOURCE */
>  #endif /* !_SYS_ENDIAN_H_ */
>

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/distrib/sets/lists/base

[Robert Elz]

Date:Sun, 8 Sep 2024 22:35:02 +
From:"Robert Elz" 
Message-ID:  <20240908223502.927a4f...@cvs.netbsd.org>

  | Modified Files:
  | src/distrib/sets/lists/base: mi
  |
  | Log Message:
  | One more mozilla-rootcerts file that is now obsolete.
  | This fixes one current build breakage, but there is more to come.

Actually, fixed all of it - the remainder I was seeing was just
the typical breakage from an update build in the distrib//cdroms
directory, where something that goes on the cdrom is altered (the
mozilla rootcerts this time, but it can be anything) in such a way
that files are added or deleted, there are no rules to cause the
cdrom spec.fs to be rebuilt in such a case, and consequently the
build fails.

We need to fix this sometime, even if it is just an "if this fails, do
a make clean, and try again" in those directories.

kre

Re: CVS commit: src/sys/dev/pci

[T K Spindler (moof)]

On Wed, Aug 21, 2024 at 01:42:28AM -0700, John Nemeth wrote:
> } Log Message:
> } Add Areca ARC-1224
> 
>  I noticed that you mentioned newer Areca devices on icb.  Is
> there a particular device that you're interested in.  I have an
> updated version of arcmsr(4) that I've been meaning to clean up
> and commit.  It has been tested with at least one device newer than
> what the in-tree code has.  It was mostly done with code that Areca
> provided with some cleanup by myself (especially the error paths).

I was interested in perhaps importing or integrating at least some of
the Areca-provided driver sources - but actual work on that on my part
has been limited to eyeballing the diffs and wondering whether there
were a cleaner way to integrate the five different variants that it
supports (and the accompanying 2x size increase in both the header and
.c files). It sounds like you're way, way further along than I am - if
you were willing and able to commit, I'd heartily encourage you to do
so.

(I do *not* have any of the newer devices in question, FWIW. I'd
also note that the FreeBSD driver for the non-SAS cards looks radically
different from arcmsr and also requires linking in a binary blob, alas.)

Re: CVS commit: src/sys/dev/pci

[John Nemeth]

On Aug 20, 22:44, "Tom Spindler" wrote:
} 
} Module Name:  src
} Committed By: dogcow
} Date: Tue Aug 20 22:44:04 UTC 2024
} 
} Modified Files:
}   src/sys/dev/pci: pcidevs
} 
} Log Message:
} Add Areca ARC-1224

Hi Tom,

 I noticed that you mentioned newer Areca devices on icb.  Is
there a particular device that you're interested in.  I have an
updated version of arcmsr(4) that I've been meaning to clean up
and commit.  It has been tested with at least one device newer than
what the in-tree code has.  It was mostly done with code that Areca
provided with some cleanup by myself (especially the error paths).

}-- End of excerpt from "Tom Spindler"

Re: CVS commit: src/lib/libc/locale

[Taylor R Campbell]

> Date: Sun, 18 Aug 2024 19:55:10 +0200
> From: Roland Illig 
> 
> Am 18.08.2024 um 15:39 schrieb Taylor R Campbell:
> > Please revert the casts and use `LINTFLAGS.mbrtoc8.c+= -X 132' instead
> > until it is fixed.
> 
> It is fixed.

Awesome, thanks!

Re: CVS commit: src/lib/libc/locale

[Roland Illig]

Am 18.08.2024 um 15:39 schrieb Taylor R Campbell:
>> Date: Sat, 17 Aug 2024 17:55:28 -0400
>> From: Christos Zoulas 
>>
>> There is only one bug. I have sent mail to Roland about it and will revert 
>> once it is fixed.
>>
>> The rest are:
>> - cast through (void *)
>> - add casts because __BITS returns uintmax_t which is wider than the 
>> destination types.
>
> To track this bug, I filed PR toolchain/58617: lint spuriously warns
> about impossible __SHIFTOUT/__BITS truncation
> (https://gnats.NetBSD.org/58617).
>
> Please revert the casts and use `LINTFLAGS.mbrtoc8.c+= -X 132' instead
> until it is fixed.

It is fixed.

Re: CVS commit: src/lib/libc/locale

[Taylor R Campbell]

> Date: Sat, 17 Aug 2024 17:55:28 -0400
> From: Christos Zoulas 
> 
> There is only one bug. I have sent mail to Roland about it and will revert 
> once it is fixed.
> 
> The rest are:
> - cast through (void *) 
> - add casts because __BITS returns uintmax_t which is wider than the 
> destination types.

To track this bug, I filed PR toolchain/58617: lint spuriously warns
about impossible __SHIFTOUT/__BITS truncation
(https://gnats.NetBSD.org/58617).

Please revert the casts and use `LINTFLAGS.mbrtoc8.c+= -X 132' instead
until it is fixed.

Re: CVS commit: src/lib/libc/locale

[Rin Okuyama]

Hi,

For __CTASSERT v.s. case label:

https://nxr.netbsd.org/xref/src/lib/libc/locale/c8rtomb.c#225

not only lint(1), but also GCC 10.5 does not accept this, which
results in build failures for platforms still using gcc.old:

https://releng.netbsd.org/builds/HEAD/202408171750Z/

Can we just drop this __CTASSERT, as the same condition is
already asserted several lines above?:

https://nxr.netbsd.org/xref/src/lib/libc/locale/c8rtomb.c#195

Thanks,
rin

Re: CVS commit: src/distrib/sets/lists/man

[Rin Okuyama]

Done :) Thanks for your kind words!

rin

On 2024/08/17 3:41, Christos Zoulas wrote:

Rin, thank you so much for working on this. LG[reat]TM. Please commit.

christos


On Aug 16, 2024, at 6:06 AM, Rin Okuyama  wrote:

Hi,

I've changed mknative-gdb to generate *.info by using makeinfo(1)
provided by pkgsrc-2024Q2:

https://github.com/IIJ-NetBSD/netbsd-src/compare/master...rokuyama:netbsd-src:exp/mknative-gdb-makeinfo

Then, pre-generated infos are installed for normal builds. Also,
in this branch:

- Add some missing documents
- Introduce MKGDBSERVER for clarity
- Similar changes for gdb.old, but continue to build *.info
  every time for normal builds

Thoughts?

Thanks,
rin

On 2024/08/16 10:02, Paul Goyette wrote:

On Thu, 15 Aug 2024, Christos Zoulas wrote:

Module Name:src
Committed By:christos
Date:Thu Aug 15 21:51:35 UTC 2024

Modified Files:
 src/distrib/sets/lists/man: mi

Log Message:
sigh, no more info files for gdb.

Can we please try and find someone to package up the info files into
pkgsrc?  It would be a real shame to not have docs...
+-+--+--+
| Paul Goyette (.sig) | PGP Key fingerprint: | E-mail addresses:|
| (Retired)   | 1B11 1849 721C 56C8 F63A | p...@whooppee.com|
| Software Developer  | 6E2E 05FD 15CE 9F2D 5102 | pgoye...@netbsd.org  |
| & Network Engineer  |  | pgoyett...@gmail.com |
+-+--+--+

Re: CVS commit: src/lib/libc/locale

[Taylor R Campbell]

> Date: Sat, 17 Aug 2024 17:55:28 -0400
> From: Christos Zoulas 
> 
> There is only one bug. I have sent mail to Roland about it and will revert 
> once it is fixed.
> 
> The rest are:
> - cast through (void *) 

There is no need to cast through (void *) -- not in C, not in KNF, and
not for any other reason I know of.  It's not a big deal to leave in,
but it's also unnecessary clutter and I would rather remove it.

> - add casts because __BITS returns uintmax_t which is wider than the 
> destination types.

I'm sympathetic to the proposition that implicit integer conversions
can conceal scary bugs in C.  But in this case, the output is
trivially guaranteed to lie within the desired range, so it is obvious
that the integer conversion never loses anything even though the
intermediate types are larger -- that's a big part of the point of the
convenience and legibility of __BITS and __SHIFTIN/__SHIFTOUT.

If lint complains because it can't figure this out, that's a lint bug.
And the casts make the code much harder to read.  Please revert this
and let's make lint serve our needs, not make us serve lint's needs.

if (pc8)
*pc8 = 0xf0 | __SHIFTOUT(c32, __BITS(20,18));
S->buf[0] = 0x80 | __SHIFTOUT(c32, __BITS(17,12));
S->buf[1] = 0x80 | __SHIFTOUT(c32, __BITS(11,6));
S->buf[2] = 0x80 | __SHIFTOUT(c32, __BITS(5,0));

versus

if (pc8)
*pc8 = (char8_t)(
0xf0 | __SHIFTOUT(c32, __BITS(20,18)));
S->buf[0] = (char8_t)(
0x80 | __SHIFTOUT(c32, __BITS(17,12)));
S->buf[1] = (char8_t)(
0x80 | __SHIFTOUT(c32, __BITS(11,6)));
S->buf[2] = (char8_t)(
0x80 | __SHIFTOUT(c32, __BITS(5,0)));

Re: CVS commit: src/lib/libc/locale

[Christos Zoulas]

There is only one bug. I have sent mail to Roland about it and will revert once 
it is fixed.

The rest are:
- cast through (void *) 
- add casts because __BITS returns uintmax_t which is wider than the 
destination types.

christos

> On Aug 17, 2024, at 5:29 PM, Taylor R Campbell  wrote:
> 
>> Module Name:src
>> Committed By:   christos
>> Date:   Sat Aug 17 20:08:13 UTC 2024
>> 
>> Modified Files:
>>src/lib/libc/locale: c16rtomb.c c8rtomb.c mbrtoc16.c mbrtoc32.c
>>mbrtoc8.c
>> 
>> Log Message:
>> pass lint, XXX see lint bug.
> 
> Can you please arrange to fix the lint bugs, and revert this clutter?

Re: CVS commit: src/lib/libc/locale

[Taylor R Campbell]

> Module Name:src
> Committed By:   christos
> Date:   Sat Aug 17 20:08:13 UTC 2024
> 
> Modified Files:
> src/lib/libc/locale: c16rtomb.c c8rtomb.c mbrtoc16.c mbrtoc32.c
> mbrtoc8.c
> 
> Log Message:
> pass lint, XXX see lint bug.

Can you please arrange to fix the lint bugs, and revert this clutter?

Re: CVS commit: src/distrib/sets/lists/man

[Steffen Nurpmeso]

Greg Troxel wrote in
 :
 |Valery Ushakov  writes:
 |
 |> On Fri, Aug 16, 2024 at 19:06:15 +0900, Rin Okuyama wrote:
 |>
 |>> I've changed mknative-gdb to generate *.info by using makeinfo(1)
 |>> provided by pkgsrc-2024Q2:
 |>> 
 |>> https://github.com/IIJ-NetBSD/netbsd-src/compare/master...rokuyama:netbs\
 |>> d-src:exp/mknative-gdb-makeinfo
 |>> 
 |>> Then, pre-generated infos are installed for normal builds.
 |>
 |> I haven't looked at the actual code, but as an emacs user I'm obviolsy
 |> all for shipping the info files.  Thanks.
 |
 |I also think we should have info files.  Not just for emacs, but info(1)
 |standalone.  It is a bug to not build the documentation, even if we have
 |to work around it needing newer info than base has, if that's what is
 |going on.

Btw *please* do join in upstream bell ringing whenever you can.
It has become one "absurdism" of our time that *release tarballs*
no longer ship with pre-generated manuals.  More and more require
superweight build infrastructures.

 --End of 

--steffen
|
|Der Kragenbaer,The moon bear,
|der holt sich munter   he cheerfully and one by one
|einen nach dem anderen runter  wa.ks himself off
|(By Robert Gernhardt)

Re: CVS commit: src/distrib/sets/lists/man

[Christos Zoulas]

Rin, thank you so much for working on this. LG[reat]TM. Please commit.

christos

> On Aug 16, 2024, at 6:06 AM, Rin Okuyama  wrote:
> 
> Hi,
> 
> I've changed mknative-gdb to generate *.info by using makeinfo(1)
> provided by pkgsrc-2024Q2:
> 
> https://github.com/IIJ-NetBSD/netbsd-src/compare/master...rokuyama:netbsd-src:exp/mknative-gdb-makeinfo
> 
> Then, pre-generated infos are installed for normal builds. Also,
> in this branch:
> 
> - Add some missing documents
> - Introduce MKGDBSERVER for clarity
> - Similar changes for gdb.old, but continue to build *.info
>  every time for normal builds
> 
> Thoughts?
> 
> Thanks,
> rin
> 
> On 2024/08/16 10:02, Paul Goyette wrote:
>> On Thu, 15 Aug 2024, Christos Zoulas wrote:
>>> Module Name:src
>>> Committed By:christos
>>> Date:Thu Aug 15 21:51:35 UTC 2024
>>> 
>>> Modified Files:
>>> src/distrib/sets/lists/man: mi
>>> 
>>> Log Message:
>>> sigh, no more info files for gdb.
>> Can we please try and find someone to package up the info files into
>> pkgsrc?  It would be a real shame to not have docs...
>> +-+--+--+
>> | Paul Goyette (.sig) | PGP Key fingerprint: | E-mail addresses:|
>> | (Retired)   | 1B11 1849 721C 56C8 F63A | p...@whooppee.com|
>> | Software Developer  | 6E2E 05FD 15CE 9F2D 5102 | pgoye...@netbsd.org  |
>> | & Network Engineer  |  | pgoyett...@gmail.com |
>> +-+--+--+

Re: CVS commit: src/distrib/sets/lists/man

[Greg Troxel]

Valery Ushakov  writes:

> On Fri, Aug 16, 2024 at 19:06:15 +0900, Rin Okuyama wrote:
>
>> I've changed mknative-gdb to generate *.info by using makeinfo(1)
>> provided by pkgsrc-2024Q2:
>> 
>> https://github.com/IIJ-NetBSD/netbsd-src/compare/master...rokuyama:netbsd-src:exp/mknative-gdb-makeinfo
>> 
>> Then, pre-generated infos are installed for normal builds.
>
> I haven't looked at the actual code, but as an emacs user I'm obviolsy
> all for shipping the info files.  Thanks.

I also think we should have info files.  Not just for emacs, but info(1)
standalone.  It is a bug to not build the documentation, even if we have
to work around it needing newer info than base has, if that's what is
going on.

Re: CVS commit: src/distrib/sets/lists/man

[Valery Ushakov]

On Fri, Aug 16, 2024 at 19:06:15 +0900, Rin Okuyama wrote:

> I've changed mknative-gdb to generate *.info by using makeinfo(1)
> provided by pkgsrc-2024Q2:
> 
> https://github.com/IIJ-NetBSD/netbsd-src/compare/master...rokuyama:netbsd-src:exp/mknative-gdb-makeinfo
> 
> Then, pre-generated infos are installed for normal builds.

I haven't looked at the actual code, but as an emacs user I'm obviolsy
all for shipping the info files.  Thanks.

-uwe

Re: CVS commit: src/distrib/sets/lists/man

[Rin Okuyama]

Hi,

I've changed mknative-gdb to generate *.info by using makeinfo(1)
provided by pkgsrc-2024Q2:

https://github.com/IIJ-NetBSD/netbsd-src/compare/master...rokuyama:netbsd-src:exp/mknative-gdb-makeinfo

Then, pre-generated infos are installed for normal builds. Also,
in this branch:

- Add some missing documents
- Introduce MKGDBSERVER for clarity
- Similar changes for gdb.old, but continue to build *.info
  every time for normal builds

Thoughts?

Thanks,
rin

On 2024/08/16 10:02, Paul Goyette wrote:

On Thu, 15 Aug 2024, Christos Zoulas wrote:


Module Name:    src
Committed By:    christos
Date:    Thu Aug 15 21:51:35 UTC 2024

Modified Files:
src/distrib/sets/lists/man: mi

Log Message:
sigh, no more info files for gdb.


Can we please try and find someone to package up the info files into
pkgsrc?  It would be a real shame to not have docs...


+-+--+--+
| Paul Goyette (.sig) | PGP Key fingerprint: | E-mail addresses:    |
| (Retired)   | 1B11 1849 721C 56C8 F63A | p...@whooppee.com    |
| Software Developer  | 6E2E 05FD 15CE 9F2D 5102 | pgoye...@netbsd.org  |
| & Network Engineer  |  | pgoyett...@gmail.com |
+-+--+--+

Re: CVS commit: src/distrib/sets/lists/man

[Paul Goyette]

On Thu, 15 Aug 2024, Christos Zoulas wrote:


Module Name:src
Committed By:   christos
Date:   Thu Aug 15 21:51:35 UTC 2024

Modified Files:
src/distrib/sets/lists/man: mi

Log Message:
sigh, no more info files for gdb.


Can we please try and find someone to package up the info files into
pkgsrc?  It would be a real shame to not have docs...


+-+--+--+
| Paul Goyette (.sig) | PGP Key fingerprint: | E-mail addresses:|
| (Retired)   | 1B11 1849 721C 56C8 F63A | p...@whooppee.com|
| Software Developer  | 6E2E 05FD 15CE 9F2D 5102 | pgoye...@netbsd.org  |
| & Network Engineer  |  | pgoyett...@gmail.com |
+-+--+--+

Re: CVS commit: src/sys

[Nick Hudson]

On 10/08/2024 22:49, Taylor R Campbell wrote:

Date: Sat, 10 Aug 2024 21:37:45 +
From: Taylor R Campbell 

I suggest doing the same here (and in bge(4) and in vmx(4) and
wherever else we have an overbroad `core lock'), with the attached
patch.  But I don't have any awge(4) hardware to test.  Could I
trouble you to test this and commit it if it works?


Oops -- missed a spot, need to take sc_mcast_lock around the call to
dwc_gmac_setmulti (and only around that call) in dwc_gmac_init.


I did some testing of this and it works so please commit.

Thanks again,
Nick

Re: CVS commit: src/sys

[Nick Hudson]

On 10/08/2024 22:37, Taylor R Campbell wrote:

Module Name:src
Committed By:   skrll
Date:   Sat Aug 10 12:16:47 UTC 2024

Modified Files:
 src/sys/arch/arm/altera: cycv_gmac.c
 src/sys/arch/arm/amlogic: meson_dwmac.c
 src/sys/arch/arm/rockchip: rk_gmac.c
 src/sys/arch/arm/sunxi: sunxi_gmac.c
 src/sys/dev/ic: dwc_gmac.c dwc_gmac_var.h

Log Message:
awge(4): MP improvements

Remove the non-MP-safe scaffolding and make the locking less
coarse.


It's great to see more MP-safety and removal of conditional-NET_MPSAFE
scaffolding -- but please, no more all-encompassing `core locks'!

The usage model for a network interface is:

1. (under IFNET_LOCK) if_init:
(a) resets hardware, while nothing else is touching registers
(b) programs multicast filter
(c) starts Tx/Rx and tick for mii/watchdog
(d) sets IFF_RUNNING

2. concurrently:
- (mii lock) periodic mii ticks or (with IFNET_LOCK too) ifmedia ioctls
- (tx lock) Tx submissions
- (rx lock) Rx notifications
- (multicast filter lock) SIOCADDMULTI/SIOCDELMULTI
- (ic lock) 802.11 state machine notifications
- (IFNET_LOCK) maybe other ioctls (some of which return ENETRESET
  to cause an if_stop/init cycle in thread context)

3. (under IFNET_LOCK) if_stop:
(a) clears IFF_RUNNING
(b) halts Tx/Rx/tick and waits for completion
(c) disables concurrent multicast updates
(d) calls mii_down to wait for concurrent mii activity to quiesce
(e) resets hardware, now that nothing is touching registers any more

All of the steps in (1) if_init, and all of the steps in (3) if_stop,
are already serialized by IFNET_LOCK, the heavyweight configuration
lock, in low-priority thread context.  Sometimes, while (2) running,
ifconfig(8) will reconfigure things, also serialized by IFNET_LOCK.

So there's _no need_ for a `core lock' to cover everything in if_init
and if_stop: IFNET_LOCK already takes care of that.

Any high-priority activity like Tx/Rx paths, callouts, &c., MUST NOT
take IFNET_LOCK or wait for the completion of the heavyweight
configuration changes, which often have to wait for concurrent
high-priority activity to cease -- such waits lead to deadlocks like
.

So it's _harmful_ to have a `core lock' cover everything in if_init
and if_stop.


Thanks for keeping me straight.

Is this documented in sys/net or somewhere else in the source tree?

Nick

Re: CVS commit: src/sys

[Taylor R Campbell]

> Date: Sat, 10 Aug 2024 21:37:45 +
> From: Taylor R Campbell 
> 
> I suggest doing the same here (and in bge(4) and in vmx(4) and
> wherever else we have an overbroad `core lock'), with the attached
> patch.  But I don't have any awge(4) hardware to test.  Could I
> trouble you to test this and commit it if it works?

Oops -- missed a spot, need to take sc_mcast_lock around the call to
dwc_gmac_setmulti (and only around that call) in dwc_gmac_init.
diff -r f339ab4bf462 sys/dev/ic/dwc_gmac.c
--- a/sys/dev/ic/dwc_gmac.c Sat Aug 10 15:20:59 2024 +
+++ b/sys/dev/ic/dwc_gmac.c Sat Aug 10 21:48:14 2024 +
@@ -87,9 +87,7 @@ static void dwc_gmac_reset_tx_ring(struc
 static void dwc_gmac_free_tx_ring(struct dwc_gmac_softc *, struct 
dwc_gmac_tx_ring *);
 static void dwc_gmac_txdesc_sync(struct dwc_gmac_softc *, int, int, int);
 static int dwc_gmac_init(struct ifnet *);
-static int dwc_gmac_init_locked(struct ifnet *);
 static void dwc_gmac_stop(struct ifnet *, int);
-static void dwc_gmac_stop_locked(struct ifnet *, int);
 static void dwc_gmac_start(struct ifnet *);
 static void dwc_gmac_start_locked(struct ifnet *);
 static int dwc_gmac_queue(struct dwc_gmac_softc *, struct mbuf *);
@@ -297,7 +295,7 @@ dwc_gmac_attach(struct dwc_gmac_softc *s
sc->sc_stopping = false;
sc->sc_txbusy = false;
 
-   sc->sc_core_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NONE);
+   sc->sc_mcast_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_SOFTNET);
sc->sc_intr_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NET);
mutex_init(&sc->sc_txq.t_mtx, MUTEX_DEFAULT, IPL_NET);
mutex_init(&sc->sc_rxq.r_mtx, MUTEX_DEFAULT, IPL_NET);
@@ -863,28 +861,13 @@ static int
 dwc_gmac_init(struct ifnet *ifp)
 {
struct dwc_gmac_softc * const sc = ifp->if_softc;
-
-   KASSERT(IFNET_LOCKED(ifp));
-
-   mutex_enter(sc->sc_core_lock);
-   int ret = dwc_gmac_init_locked(ifp);
-   mutex_exit(sc->sc_core_lock);
-
-   return ret;
-}
-
-static int
-dwc_gmac_init_locked(struct ifnet *ifp)
-{
-   struct dwc_gmac_softc * const sc = ifp->if_softc;
uint32_t ffilt;
 
ASSERT_SLEEPABLE();
KASSERT(IFNET_LOCKED(ifp));
-   KASSERT(mutex_owned(sc->sc_core_lock));
KASSERT(ifp == &sc->sc_ec.ec_if);
 
-   dwc_gmac_stop_locked(ifp, 0);
+   dwc_gmac_stop(ifp, 0);
 
/*
 * Configure DMA burst/transfer mode and RX/TX priorities.
@@ -914,7 +897,9 @@ dwc_gmac_init_locked(struct ifnet *ifp)
/*
 * Set up multicast filter
 */
+   mutex_enter(sc->sc_mcast_lock);
dwc_gmac_setmulti(sc);
+   mutex_exit(sc->sc_mcast_lock);
 
/*
 * Set up dma pointer for RX and TX ring
@@ -942,11 +927,11 @@ dwc_gmac_init_locked(struct ifnet *ifp)
 
mutex_enter(sc->sc_intr_lock);
sc->sc_stopping = false;
+   mutex_exit(sc->sc_intr_lock);
 
mutex_enter(&sc->sc_txq.t_mtx);
sc->sc_txbusy = false;
mutex_exit(&sc->sc_txq.t_mtx);
-   mutex_exit(sc->sc_intr_lock);
 
return 0;
 }
@@ -1015,29 +1000,23 @@ dwc_gmac_stop(struct ifnet *ifp, int dis
 {
struct dwc_gmac_softc * const sc = ifp->if_softc;
 
-   mutex_enter(sc->sc_core_lock);
-   dwc_gmac_stop_locked(ifp, disable);
-   mutex_exit(sc->sc_core_lock);
-}
-
-static void
-dwc_gmac_stop_locked(struct ifnet *ifp, int disable)
-{
-   struct dwc_gmac_softc * const sc = ifp->if_softc;
-
ASSERT_SLEEPABLE();
KASSERT(IFNET_LOCKED(ifp));
-   KASSERT(mutex_owned(sc->sc_core_lock));
+
+   ifp->if_flags &= ~IFF_RUNNING;
+
+   mutex_enter(sc->sc_mcast_lock);
+   sc->sc_if_flags = ifp->if_flags;
+   mutex_exit(sc->sc_mcast_lock);
 
mutex_enter(sc->sc_intr_lock);
sc->sc_stopping = true;
+   mutex_exit(sc->sc_intr_lock);
 
mutex_enter(&sc->sc_txq.t_mtx);
sc->sc_txbusy = false;
mutex_exit(&sc->sc_txq.t_mtx);
 
-   mutex_exit(sc->sc_intr_lock);
-
bus_space_write_4(sc->sc_bst, sc->sc_bsh,
AWIN_GMAC_DMA_OPMODE,
bus_space_read_4(sc->sc_bst, sc->sc_bsh,
@@ -1051,9 +1030,6 @@ dwc_gmac_stop_locked(struct ifnet *ifp, 
mii_down(&sc->sc_mii);
dwc_gmac_reset_tx_ring(sc, &sc->sc_txq);
dwc_gmac_reset_rx_ring(sc, &sc->sc_rxq);
-
-   ifp->if_flags &= ~IFF_RUNNING;
-   sc->sc_if_flags = ifp->if_flags;
 }
 
 /*
@@ -1150,7 +1126,7 @@ dwc_gmac_ifflags_cb(struct ethercom *ec)
int ret = 0;
 
KASSERT(IFNET_LOCKED(ifp));
-   mutex_enter(sc->sc_core_lock);
+   mutex_enter(sc->sc_mcast_lock);
 
u_short change = ifp->if_flags ^ sc->sc_if_flags;
sc->sc_if_flags = ifp->if_flags;
@@ -1161,7 +1137,7 @@ dwc_gmac_ifflags_cb(struct ethercom *ec)
dwc_gmac_setmulti(sc);
}
 
-   mutex_exit(sc->sc_core_lock);
+   mutex_exit(sc->sc_mcast_lock);
 
return ret;
 }
@@ -1187,7 +1163,7 @@ dwc_gmac_ioctl(struct ifnet *ifp, u_long

Re: CVS commit: src/sys

[Taylor R Campbell]

> Module Name:src
> Committed By:   skrll
> Date:   Sat Aug 10 12:16:47 UTC 2024
> 
> Modified Files:
> src/sys/arch/arm/altera: cycv_gmac.c
> src/sys/arch/arm/amlogic: meson_dwmac.c
> src/sys/arch/arm/rockchip: rk_gmac.c
> src/sys/arch/arm/sunxi: sunxi_gmac.c
> src/sys/dev/ic: dwc_gmac.c dwc_gmac_var.h
> 
> Log Message:
> awge(4): MP improvements
> 
> Remove the non-MP-safe scaffolding and make the locking less
> coarse.

It's great to see more MP-safety and removal of conditional-NET_MPSAFE
scaffolding -- but please, no more all-encompassing `core locks'!

The usage model for a network interface is:

1. (under IFNET_LOCK) if_init:
   (a) resets hardware, while nothing else is touching registers
   (b) programs multicast filter
   (c) starts Tx/Rx and tick for mii/watchdog
   (d) sets IFF_RUNNING

2. concurrently:
   - (mii lock) periodic mii ticks or (with IFNET_LOCK too) ifmedia ioctls
   - (tx lock) Tx submissions
   - (rx lock) Rx notifications
   - (multicast filter lock) SIOCADDMULTI/SIOCDELMULTI
   - (ic lock) 802.11 state machine notifications
   - (IFNET_LOCK) maybe other ioctls (some of which return ENETRESET
 to cause an if_stop/init cycle in thread context)

3. (under IFNET_LOCK) if_stop:
   (a) clears IFF_RUNNING
   (b) halts Tx/Rx/tick and waits for completion
   (c) disables concurrent multicast updates
   (d) calls mii_down to wait for concurrent mii activity to quiesce
   (e) resets hardware, now that nothing is touching registers any more

All of the steps in (1) if_init, and all of the steps in (3) if_stop,
are already serialized by IFNET_LOCK, the heavyweight configuration
lock, in low-priority thread context.  Sometimes, while (2) running,
ifconfig(8) will reconfigure things, also serialized by IFNET_LOCK.

So there's _no need_ for a `core lock' to cover everything in if_init
and if_stop: IFNET_LOCK already takes care of that.

Any high-priority activity like Tx/Rx paths, callouts, &c., MUST NOT
take IFNET_LOCK or wait for the completion of the heavyweight
configuration changes, which often have to wait for concurrent
high-priority activity to cease -- such waits lead to deadlocks like
.

So it's _harmful_ to have a `core lock' cover everything in if_init
and if_stop.

In usbnet(9), I broke the deadlock by eliminating the core lock and
replacing it by a narrowly scoped lock covering only the state needed
by multicast filter updates, which happen from high-priority contexts.
I suggest doing the same here (and in bge(4) and in vmx(4) and
wherever else we have an overbroad `core lock'), with the attached
patch.  But I don't have any awge(4) hardware to test.  Could I
trouble you to test this and commit it if it works?
diff -r f339ab4bf462 sys/dev/ic/dwc_gmac.c
--- a/sys/dev/ic/dwc_gmac.c Sat Aug 10 15:20:59 2024 +
+++ b/sys/dev/ic/dwc_gmac.c Sat Aug 10 21:19:37 2024 +
@@ -87,9 +87,7 @@ static void dwc_gmac_reset_tx_ring(struc
 static void dwc_gmac_free_tx_ring(struct dwc_gmac_softc *, struct 
dwc_gmac_tx_ring *);
 static void dwc_gmac_txdesc_sync(struct dwc_gmac_softc *, int, int, int);
 static int dwc_gmac_init(struct ifnet *);
-static int dwc_gmac_init_locked(struct ifnet *);
 static void dwc_gmac_stop(struct ifnet *, int);
-static void dwc_gmac_stop_locked(struct ifnet *, int);
 static void dwc_gmac_start(struct ifnet *);
 static void dwc_gmac_start_locked(struct ifnet *);
 static int dwc_gmac_queue(struct dwc_gmac_softc *, struct mbuf *);
@@ -297,7 +295,7 @@ dwc_gmac_attach(struct dwc_gmac_softc *s
sc->sc_stopping = false;
sc->sc_txbusy = false;
 
-   sc->sc_core_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NONE);
+   sc->sc_mcast_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_SOFTNET);
sc->sc_intr_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NET);
mutex_init(&sc->sc_txq.t_mtx, MUTEX_DEFAULT, IPL_NET);
mutex_init(&sc->sc_rxq.r_mtx, MUTEX_DEFAULT, IPL_NET);
@@ -863,28 +861,13 @@ static int
 dwc_gmac_init(struct ifnet *ifp)
 {
struct dwc_gmac_softc * const sc = ifp->if_softc;
-
-   KASSERT(IFNET_LOCKED(ifp));
-
-   mutex_enter(sc->sc_core_lock);
-   int ret = dwc_gmac_init_locked(ifp);
-   mutex_exit(sc->sc_core_lock);
-
-   return ret;
-}
-
-static int
-dwc_gmac_init_locked(struct ifnet *ifp)
-{
-   struct dwc_gmac_softc * const sc = ifp->if_softc;
uint32_t ffilt;
 
ASSERT_SLEEPABLE();
KASSERT(IFNET_LOCKED(ifp));
-   KASSERT(mutex_owned(sc->sc_core_lock));
KASSERT(ifp == &sc->sc_ec.ec_if);
 
-   dwc_gmac_stop_locked(ifp, 0);
+   dwc_gmac_stop(ifp, 0);
 
/*
 * Configure DMA burst/transfer mode and RX/TX priorities.
@@ -942,11 +925,11 @@ dwc_gmac_init_locked(struct ifnet *ifp)
 
mutex_enter(sc->sc_intr_lock);
sc->sc_stopping = false;
+   mutex_exit(sc->sc_intr_lock);
 
mutex_enter(&sc->sc_txq.t_mt

Re: CVS commit: src/share/man/man5

[Christos Zoulas]

Fixed,

christos

> On Aug 4, 2024, at 6:52 PM, Valery Ushakov  wrote:
> 
> On Sun, Aug 04, 2024 at 13:18:01 -0400, Christos Zoulas wrote:
> 
>> It should say acct_process(9), but we don't have a man page for it
>> (yet).
> 
> A dangling reference is much better than the current incorrect and
> confusing wording.  Please, can you fix it?
> 
> 
>>> On Aug 4, 2024, at 1:02 PM, Valery Ushakov  wrote:
>>> 
>>> On Sat, Aug 03, 2024 at 19:49:06 -0400, Christos Zoulas wrote:
>>> 
 Modified Files:
src/share/man/man5: acct.5
 
 Log Message:
 PR/58515: Kouichi Hashikawa: Sync with the header file
>>> 
>>> I notice that the man page says:
>>> 
>>>... If a process terminates, and accounting is enabled, the kernel
>>>calls the acct(2) function call to prepare and append the record to the
>>>accounting file.
>>> 
>>> which doesn't make any sense to me, as acct(2) is a syscall that is
>>> called by superuser once to enable accounting.  Any ideas what it's
>>> meant to be said here?
>>> 
>>> -uwe
> 
> -uwe

Re: CVS commit: src/share/man/man5

[Valery Ushakov]

On Sun, Aug 04, 2024 at 13:18:01 -0400, Christos Zoulas wrote:

> It should say acct_process(9), but we don't have a man page for it
> (yet).

A dangling reference is much better than the current incorrect and
confusing wording.  Please, can you fix it?


> > On Aug 4, 2024, at 1:02 PM, Valery Ushakov  wrote:
> > 
> > On Sat, Aug 03, 2024 at 19:49:06 -0400, Christos Zoulas wrote:
> > 
> >> Modified Files:
> >>src/share/man/man5: acct.5
> >> 
> >> Log Message:
> >> PR/58515: Kouichi Hashikawa: Sync with the header file
> > 
> > I notice that the man page says:
> > 
> > ... If a process terminates, and accounting is enabled, the kernel
> > calls the acct(2) function call to prepare and append the record to the
> > accounting file.
> > 
> > which doesn't make any sense to me, as acct(2) is a syscall that is
> > called by superuser once to enable accounting.  Any ideas what it's
> > meant to be said here?
> > 
> > -uwe

-uwe

Re: CVS commit: src/share/man/man5

[Christos Zoulas]

It should say acct_process(9), but we don't have a man page for it (yet).

christos

> On Aug 4, 2024, at 1:02 PM, Valery Ushakov  wrote:
> 
> On Sat, Aug 03, 2024 at 19:49:06 -0400, Christos Zoulas wrote:
> 
>> Modified Files:
>>  src/share/man/man5: acct.5
>> 
>> Log Message:
>> PR/58515: Kouichi Hashikawa: Sync with the header file
> 
> I notice that the man page says:
> 
> ... If a process terminates, and accounting is enabled, the kernel
> calls the acct(2) function call to prepare and append the record to the
> accounting file.
> 
> which doesn't make any sense to me, as acct(2) is a syscall that is
> called by superuser once to enable accounting.  Any ideas what it's
> meant to be said here?
> 
> -uwe

re: CVS commit: src/external/mit/xorg/server/drivers/xf86-video-nv

[matthew green]

"Taylor R Campbell" writes:
> Module Name:  src
> Committed By: riastradh
> Date: Tue Jul 23 07:52:06 UTC 2024
>
> Modified Files:
>   src/external/mit/xorg/server/drivers/xf86-video-nv: Makefile
>
> Log Message:
> xf86-video-nv: Don't fail on -Wpointer-sign with clang.
>
> Not sure why this isn't disabled by default (surely this very old
> code hasn't been clean for it in a long time?), but with any luck
> this should resolve clang build failures like:

thanks.

may as well revert the attempt(s) to avoid this warning in the
code now.

it was updated recently, actually, and this specific line changed
in upstream and needed a merge, and then i had one attempt to fix
the warning that clearly didn't work.


.mrg.

Re: CVS commit: src/external/historical/nawk/dist

[Steffen Nurpmeso]

Christos Zoulas wrote in
 <20240721194847.c8c05f...@cvs.netbsd.org>:
 |Module Name:  src
 |Committed By: christos
 |Date: Sun Jul 21 19:48:47 UTC 2024
 |
 |Modified Files:
 | src/external/historical/nawk/dist: lib.c
 |
 |Log Message:
 |PR/58421: RVP: fix readdir on tmpfs. Upstream merge is complicated \
 |now because
 |the bsd branch has not been updated in ages.

Hmm, is this the "real" aka Kernighan's nawk?  Then upstream now
is at https://github.com/onetrueawk/awk.git, with Aharon Robbins
of Gawk having spent lots of time on it, and Kernighan himself
adding some UTF-8 support (which, pssst, had quite some bugs fixed
by Robbins and Millert as of sudo aka OpenBSD), and i maintain
a package for the BSD branch, which is up-to-date, via

  name=nawk
  version=20240623-bsd
  realn=awk
  gitver=9f971fba471ba561af661694ea55769b2b914988
  release=1
  
source=(https://github.com/onetrueawk/${realn}/archive/${gitver}/${realn}-${version}.tar.gz)

  build() {
  cd ${realn}-${gitver}

  # Fix term sequence mess in "box-ports -> fakeroot pkgmk" container
  sed -Ei'' 's/^YACC .+$/YACC = bison -d --color=no/' makefile

  make
  install -D a.out "${PKG}"/usr/bin/${name}
  install -D -m 0644 awk.1 "${PKG}"/usr/share/man/man1/${name}.1
  }

 --End of <20240721194847.c8c05f...@cvs.netbsd.org>

--steffen
|
|Der Kragenbaer,The moon bear,
|der holt sich munter   he cheerfully and one by one
|einen nach dem anderen runter  wa.ks himself off
|(By Robert Gernhardt)
|
| Only during dog days:
| On the 81st anniversary of the Goebbel's Sportpalast speech
| von der Leyen gave an overlong hypocritical inauguration one.
| The brew's essence of our civilizing advancement seems o be:
|   Total war - shortest war -> Permanent war - everlasting war

Re: CVS commit: src/etc

[Havard Eidnes]

>> Module Name:src
>> Committed By:   he
>> Date:   Sun Jul 21 14:56:16 UTC 2024
>> 
>> Modified Files:
>> src/etc: security
>> 
>> Log Message:
>> etc/security: emit proper error message when there are dup groups.
>> 
>> ...instead of erroring with "[: $grpname: unexpected operator".
>
> Nice, is there a PR for this?  Should we pull it up to 10 or 9?

Sorry, no PR, I thought the obviousness of the two-character
addition would not warrant that much machinery.

And, yes, this is probably pull-up material.

When I'm back from vacation this coming monday I'll follow up
Robert's comment about there being more quoting bugs in that
script, and we'll see if I can spot them.

Regards,

- Håvard

Re: CVS commit: src/etc

[Robert Elz]

Date:Mon, 22 Jul 2024 00:07:34 +
From:Taylor R Campbell 
Message-ID:  <20240722000735.3069360...@jupiter.mumble.net>

  | Nice, is there a PR for this?  Should we pull it up to 10 or 9?

There are lots more sh quoting issues in /etc/security - normally they're
not a problem..

kre

Re: CVS commit: src/etc

[Taylor R Campbell]

> Module Name:src
> Committed By:   he
> Date:   Sun Jul 21 14:56:16 UTC 2024
> 
> Modified Files:
> src/etc: security
> 
> Log Message:
> etc/security: emit proper error message when there are dup groups.
> 
> ...instead of erroring with "[: $grpname: unexpected operator".

Nice, is there a PR for this?  Should we pull it up to 10 or 9?

Re: CVS commit: src/tests/lib/libc/c063

[Christos Zoulas]

In article ,
Roland Illig   wrote:
>Am 10.07.2024 um 03:12 schrieb Christos Zoulas:
>
>src/tests/lib/libc/c063/t_fchmodat.c
>> -ATF_REQUIRE(st.st_mode = 0600);
>> +ATF_REQUIRE(st.st_mode == 0600);
>
>Should we do something to detect bugs like these mechanically?
>
>ATF_REQUIRE(cond) currently expands to "if (!(cond))", and I guess due
>to the parentheses around "cond", GCC does not warn about the assignment
>in the condition.
>
>One idea is to rewrite ATF_REQUIRE to be "if (cond) {} else", which
>would make GCC complain directly.
>
>Another idea is to make lint warn about the pattern "!(a = b)", even
>though its output is not looked upon by many people.

Yes, I was thinking along the same lines. If the gcc construct does not
produce warnings, I would go with that.

christos

Re: CVS commit: src/tests/lib/libc/c063

[Roland Illig]

Am 10.07.2024 um 03:12 schrieb Christos Zoulas:

src/tests/lib/libc/c063/t_fchmodat.c
> - ATF_REQUIRE(st.st_mode = 0600);
> + ATF_REQUIRE(st.st_mode == 0600);

Should we do something to detect bugs like these mechanically?

ATF_REQUIRE(cond) currently expands to "if (!(cond))", and I guess due
to the parentheses around "cond", GCC does not warn about the assignment
in the condition.

One idea is to rewrite ATF_REQUIRE to be "if (cond) {} else", which
would make GCC complain directly.

Another idea is to make lint warn about the pattern "!(a = b)", even
though its output is not looked upon by many people.

Roland

Re: CVS commit: src/sys/kern

[Taylor R Campbell]

> Module Name:src
> Committed By:   mrg
> Date:   Thu Jul  4 05:59:05 UTC 2024
> 
> Modified Files:
> src/sys/kern: vfs_syscalls.c
> 
> Log Message:
> don't fd_putfile() if you haven't grabbed a ref already.
> 
> the condition to call fd_getvnode() was changed, but the condition
> to call fd_putfile() afterwards was not changed, leading to a panic
> seen by Chavdar on current-users, probably.
> 
> builds, runs, seems obvious.

The automatic testbed is failing to run tests to completion now:

https://releng.netbsd.org/b5reports/i386/commits-2024.07.html#2024.07.04.05.59.05

Our sloppy process for fixing the vfs_syscalls.c issue is obviously
failing now, after days of flailing around with band-aids.  I propose
to back out all of the recent changes:

https://mail-index.netbsd.org/source-changes/2024/06/29/msg152020.html
https://mail-index.netbsd.org/source-changes/2024/07/01/msg152081.html
https://mail-index.netbsd.org/source-changes/2024/07/01/msg152082.html
https://mail-index.netbsd.org/source-changes/2024/07/01/msg152083.html
https://mail-index.netbsd.org/source-changes/2024/07/04/msg152216.html

And then redo them with:

1. A PR that explains the problem, with references, and can track the
   changes in case we need to pull them up.

2. Posting the patch for public review first.

3. Adding automatic tests that exercise all the relevant cases and
   xfail, but that would pass with the fix.

4. _Then_ committing the fix.

Re: CVS commit: src/sys/external/bsd/drm2/amdgpu

[Ryo ONODERA]

On July 3, 2024 5:08:51 AM GMT+09:00, Taylor R Campbell  
wrote:
>Module Name:   src
>Committed By:  riastradh
>Date:  Tue Jul  2 20:08:51 UTC 2024
>
>Modified Files:
>   src/sys/external/bsd/drm2/amdgpu: amdgpu2netbsd
>
>Log Message:
>amdgpu: Update amdgpu2netbsd to prepare for new import.
>
>
>To generate a diff of this commit:
>cvs rdiff -u -r1.4 -r1.5 src/sys/external/bsd/drm2/amdgpu/amdgpu2netbsd
>
>Please note that diffs are not public domain; they are subject to the
>copyright notices on the relevant files.
>

Hi,

I am happy to see the drmkms support from Linux 6.6 in the near future.
Almost all my faster laptops require newer drmkms support.

Thank you.
-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/sys/arch/sun2/conf

[Rin Okuyama]

Hi,

On 2024/06/30 16:18, matthew green wrote:

"Rin Okuyama" writes:

Module Name:src
Committed By:   rin
Date:   Sun Jun 30 05:59:14 UTC 2024

Modified Files:
src/sys/arch/sun2/conf: GENERIC

Log Message:
sun2: GENERIC: XXX: Drop `MODULAR` and `compat_netbsd16.config`

as a workaround for memory shortage. Even with 8MB-RAM configuration,
live-image for TME gets stalled during processing /etc/rc, without
any response to serial console :(

With this change, it can boot into multi-user mode, at least.

XXX
Other config files? Make more kernel components optional?


i'm surprised MODULAR was here!  huh.


I added it few years ago, when the biggest problem was kernel size
itself; sun2 firmware is not capable for kernel image >= 2MB. So,
I "replaced" many options with `MODULAR`.

However, today, memory starvation takes place even if kernel fits
within 2MB :(


this kernel is already very cut down, reducing further is
likely what you say - need to make more optional parts.


Agreed. Also, we need to reduce runtime memory consumption.

Thanks,
rin


thanks.


.mrg.

Re: CVS commit: src/sys/dev/usb

[Taylor R Campbell]

> Module Name:src
> Committed By:   riastradh
> Date:   Sun Jun 30 16:35:19 UTC 2024
> 
> Modified Files:
> src/sys/dev/usb: if_url.c
> 
> Log Message:
> url(4): uint32_t for 32-bit hash so h>>31 becomes 0/1, not +1/-1.

That was supposed to read:

url(4): uint32_t for 32-bit hash so h>>31 becomes 0/1, not 0/-1.

re: CVS commit: src/sys/arch/sun2/conf

[matthew green]

"Rin Okuyama" writes:
> Module Name:  src
> Committed By: rin
> Date: Sun Jun 30 05:59:14 UTC 2024
>
> Modified Files:
>   src/sys/arch/sun2/conf: GENERIC
>
> Log Message:
> sun2: GENERIC: XXX: Drop `MODULAR` and `compat_netbsd16.config`
>
> as a workaround for memory shortage. Even with 8MB-RAM configuration,
> live-image for TME gets stalled during processing /etc/rc, without
> any response to serial console :(
>
> With this change, it can boot into multi-user mode, at least.
>
> XXX
> Other config files? Make more kernel components optional?

i'm surprised MODULAR was here!  huh.

this kernel is already very cut down, reducing further is
likely what you say - need to make more optional parts.

thanks.


.mrg.

Re: CVS commit: src/share/mk

[Roland Illig]

Am 28.06.2024 um 22:45 schrieb Taylor R Campbell:
> +# It is tempting to use the make :P modifier here so that you can just
> +# write
> +#
> +#VERSION_MAP=foo.map
> +#
> +# instead of
> +#
> +#VERSION_MAP=${.CURDIR}/foo.map
> +#
> +# but it appears that :P works _only_ with literal names, not with
> +# expansions, so while you could write
> +#
> +#VERSION_MAP=${foo.map:P}
> +#
> +# in the makefile, we can't set
> +#
> +#LDFLAGS+=   -Wl,--version-script=${${VERSION_MAP}:P}
> +#
> +# here.

The ':P' modifier treats a literal in the same way as an expression, so
there's no difference between ${foo.map:P} and ${${:Ufoo.map}:P}, as in
both cases the "variable name" is 'foo.map'. If you still remember how
you came to that conclusion, I'd be interested in it.

In order to make the ':P' modifier work, the "variable name" must be an
existing node in the dependency tree, at the point where the ':P'
modifier is evaluated.

Here's a demonstration:

> .MAKEFLAGS: -dv
> .PATH: /dev
> VERSION_MAP=  null
> Resolve_via_Path: .PHONY .NOTMAIN ${VERSION_MAP}
> LDFLAGS+= -Wl,--version-script=${${VERSION_MAP}:P}
> LDFLAGS+= -Wl,--version-script=${null:P}
> .info ${LDFLAGS}
> .MAKEFLAGS: -dF/dev/null
> all:

The 'Resolve_via_Path' dependency line adds the VERSION_MAP file to the
dependency tree. Instead of 'Resolve_via_Path', you can choose any other
name or even leave it empty, making the line start with a "magic colon".

Roland

Re: CVS commit: src/external/mit/libcbor/lib

[Jörg Sonnenberger]

On Saturday, June 22, 2024 3:36:54 PM GMT+2 Taylor R Campbell wrote:
> Module Name:  src
> Committed By: riastradh
> Date: Sat Jun 22 13:36:54 UTC 2024
> 
> Modified Files:
>   src/external/mit/libcbor/lib: Makefile
> 
> Log Message:
> libcbor: Fix make dependencies on configuration.h.
> 
> PR lib/58359
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.3 -r1.4 src/external/mit/libcbor/lib/Makefile

The correct approach is IMO to use DPADD to ensure that the file
is generated in the depends phase and let regular *.d do the rest
for any rebuilds.

Joerg

Re: CVS commit: src/tools/gcc

[Greg Troxel]

Taylor R Campbell  writes:

> How is the host C++ compiler supposed to know that we are asking it to
> compile C++11 and not C++98 or C++17?

The compilation line should pass --std=c++11.  Arguably, any program
that invokes a compiler and does not pass a --std is buggy.  Things only
mostly work because mostly --std=c++17 will compile C++11 programs, and
the tendency is for later compiler versions to have higher default
languages.

In pkgsrc, we have a way to add --std=foo via the wrappers,  to handle
packages that don't do this right (generally, at all).

Is that what you meant, or something else?

Re: CVS commit: src/tools/gcc

[Taylor R Campbell]

> Date: Mon, 17 Jun 2024 03:01:55 +1000
> from: matthew green 
> 
> "David H. Gutteridge" writes:
> > Module Name:src
> > Committed By:   gutteridge
> > Date:   Sun Jun 16 16:03:30 UTC 2024
> >
> > tools/gcc/Makefile: force std=c++11 for GCC 12 builds
> >
> > GCC >= 11 now requires C++11 to build. Impacted individual components
> > vary whether they test for if std=c++11 is the default or has to be
> > passed. Regardless, our build infrastructure applies its own overrides.
> > Force this setting so that we can still build on NetBSD 8.x hosts (and
> > probably 7.x, too). Build tested on NetBSD 8.3.
> 
> i'm not sure i like this.  the build environment is expected to
> provide a valid compiler, and the above assumes how the compiler
> works.
> 
> if we really want to support this without user-env setup, then
> i think a confiure test (from tools/compat) would be better than
> forcing this option here directly.
> 
> (also note this is not the first time this sort of issue has
> arrived, and we always asked the build-env provider to fix it,
> but i'm not against eg, the configure test idea.)

How is the host C++ compiler supposed to know that we are asking it to
compile C++11 and not C++98 or C++17?

gcc's configure scripts search for a way to pass this information to
the host compiler (e.g., AX_CXX_COMPILE_STDCXX).  I'm fuzzy on how
these are supposed to work but it looks like maybe the existing
configure tests are not getting used for some reason.

Re: CVS commit: src/tools/gcc

[David H. Gutteridge]

On 2024-06-16 13:01, matthew green wrote:

"David H. Gutteridge" writes:


Module Name:src
Committed By:gutteridge
Date:Sun Jun 16 16:03:30 UTC 2024

Modified Files:
src/tools/gcc: Makefile

Log Message:
tools/gcc/Makefile: force std=c++11 for GCC 12 builds

GCC >= 11 now requires C++11 to build. Impacted individual components
vary whether they test for if std=c++11 is the default or has to be
passed. Regardless, our build infrastructure applies its own 
overrides.

Force this setting so that we can still build on NetBSD 8.x hosts (and
probably 7.x, too). Build tested on NetBSD 8.3.


i'm not sure i like this.  the build environment is expected to
provide a valid compiler, and the above assumes how the compiler
works.

if we really want to support this without user-env setup, then
i think a confiure test (from tools/compat) would be better than
forcing this option here directly.

(also note this is not the first time this sort of issue has
arrived, and we always asked the build-env provider to fix it,
but i'm not against eg, the configure test idea.)


I can certainly simply revert for now, if you prefer.

I'm a little confused by your answer, as there are pre-existing bits
that already do this or something similar. (Well, or intend to.)

tools/isl/Makefile has:

# XXX this gets lost some how.  Not portable.
HOST_CXXFLAGS+= -std=gnu++17

A different example is from tools/Makefile.gmakehost:

# Disable use of pre-compiled headers on Darwin.
BUILD_OSTYPE!= uname -s
HOST_COMPILER_CLANG!= if ${HOST_CC} --version | grep -q -s clang; then 
echo yes; else echo no; fi

.if ${HOST_COMPILER_CLANG} == "yes"
HOST_CFLAGS+=-O2 -no-cpp-precomp
.endif

The latter is intended to deal with a different sort of issue, but I
see it as being related in terms of reaching over and injecting its
own HOST_* values. If we say "the host should sort out its own issues",
effectively, then I don't see why that would be there, either. (There
is more like that elsewhere.)

I realize I supplied a "GCC-style" argument that not all compilers
will accept. I don't know what compilers would be viable now other than
GCC and Clang?

The goal is to be able to set different compiler flags on a per
component basis. There could be other components that don't want the
same standard forced (or whatever). To your point, we could have
something that tests and sets a make variable that's conditionally
applied down in a particular component.

Regards,

Dave

re: CVS commit: src/tools/gcc

[matthew green]

"David H. Gutteridge" writes:
> Module Name:  src
> Committed By: gutteridge
> Date: Sun Jun 16 16:03:30 UTC 2024
>
> Modified Files:
>   src/tools/gcc: Makefile
>
> Log Message:
> tools/gcc/Makefile: force std=c++11 for GCC 12 builds
>
> GCC >= 11 now requires C++11 to build. Impacted individual components
> vary whether they test for if std=c++11 is the default or has to be
> passed. Regardless, our build infrastructure applies its own overrides.
> Force this setting so that we can still build on NetBSD 8.x hosts (and
> probably 7.x, too). Build tested on NetBSD 8.3.

i'm not sure i like this.  the build environment is expected to
provide a valid compiler, and the above assumes how the compiler
works.

if we really want to support this without user-env setup, then
i think a confiure test (from tools/compat) would be better than
forcing this option here directly.

(also note this is not the first time this sort of issue has
arrived, and we always asked the build-env provider to fix it,
but i'm not against eg, the configure test idea.)

thanks.


.mrg.

Re: CVS commit: src/sys/dev/fdt

[Rin Okuyama]

Thank you, too, for clarification!

rin

On 2024/06/13 5:10, Nick Hudson wrote:

Thanks for the fix.

The bug affects enable and disable in the all (-1) indexes case and
references out of bounds data.

Nick

On 12/06/2024 08:36, Rin Okuyama wrote:

Hmm, there was a confusion for my side.

This bug affected cases where

(1) index >= 1 is explicitly specified for
fdtbus_powerdomain_enable_index(),

as well as

(2) all indices are implicitly specified by
fdtbus_powerdomain_enable().

s/enable/disable/ functions were affected also.

Thanks,
rin

On 2024/06/12 15:39, Rin Okuyama wrote:

Module Name:    src
Committed By:    rin
Date:    Wed Jun 12 06:39:28 UTC 2024

Modified Files:
src/sys/dev/fdt: fdt_powerdomain.c

Log Message:
fdt_powerdomain: Fix bug by which pd index >= 1 couldn't be enabled

Length in bytes was mistakenly used as number of uint32_t variables.


To generate a diff of this commit:
cvs rdiff -u -r1.1 -r1.2 src/sys/dev/fdt/fdt_powerdomain.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.


Modified files:

Index: src/sys/dev/fdt/fdt_powerdomain.c
diff -u src/sys/dev/fdt/fdt_powerdomain.c:1.1
src/sys/dev/fdt/fdt_powerdomain.c:1.2
--- src/sys/dev/fdt/fdt_powerdomain.c:1.1    Fri Mar  4 08:19:06 2022
+++ src/sys/dev/fdt/fdt_powerdomain.c    Wed Jun 12 06:39:28 2024
@@ -1,4 +1,4 @@
-/* $NetBSD: fdt_powerdomain.c,v 1.1 2022/03/04 08:19:06 skrll Exp $ */
+/* $NetBSD: fdt_powerdomain.c,v 1.2 2024/06/12 06:39:28 rin Exp $ */
  /*-
   * Copyright (c) 2022 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
   */
  #include 
-__KERNEL_RCSID(0, "$NetBSD: fdt_powerdomain.c,v 1.1 2022/03/04
08:19:06 skrll Exp $");
+__KERNEL_RCSID(0, "$NetBSD: fdt_powerdomain.c,v 1.2 2024/06/12
06:39:28 rin Exp $");
  #include 
@@ -103,7 +103,7 @@ fdtbus_powerdomain_enable_internal(int p
  if (pds == NULL)
  return EINVAL;
-    for (const uint32_t *pd = pds; pd < pds + len; index--) {
+    for (const uint32_t *pd = pds; pd < pds + len / sizeof(*pd);
index--) {
  uint32_t pd_node =
 fdtbus_get_phandle_from_native(be32toh(pd[0]));
  struct fdtbus_powerdomain_controller *pdc =

Re: CVS commit: src/sys/dev/fdt

[Nick Hudson]

Thanks for the fix.

The bug affects enable and disable in the all (-1) indexes case and
references out of bounds data.

Nick

On 12/06/2024 08:36, Rin Okuyama wrote:

Hmm, there was a confusion for my side.

This bug affected cases where

(1) index >= 1 is explicitly specified for
fdtbus_powerdomain_enable_index(),

as well as

(2) all indices are implicitly specified by
fdtbus_powerdomain_enable().

s/enable/disable/ functions were affected also.

Thanks,
rin

On 2024/06/12 15:39, Rin Okuyama wrote:

Module Name:    src
Committed By:    rin
Date:    Wed Jun 12 06:39:28 UTC 2024

Modified Files:
src/sys/dev/fdt: fdt_powerdomain.c

Log Message:
fdt_powerdomain: Fix bug by which pd index >= 1 couldn't be enabled

Length in bytes was mistakenly used as number of uint32_t variables.


To generate a diff of this commit:
cvs rdiff -u -r1.1 -r1.2 src/sys/dev/fdt/fdt_powerdomain.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.


Modified files:

Index: src/sys/dev/fdt/fdt_powerdomain.c
diff -u src/sys/dev/fdt/fdt_powerdomain.c:1.1
src/sys/dev/fdt/fdt_powerdomain.c:1.2
--- src/sys/dev/fdt/fdt_powerdomain.c:1.1    Fri Mar  4 08:19:06 2022
+++ src/sys/dev/fdt/fdt_powerdomain.c    Wed Jun 12 06:39:28 2024
@@ -1,4 +1,4 @@
-/* $NetBSD: fdt_powerdomain.c,v 1.1 2022/03/04 08:19:06 skrll Exp $ */
+/* $NetBSD: fdt_powerdomain.c,v 1.2 2024/06/12 06:39:28 rin Exp $ */
  /*-
   * Copyright (c) 2022 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
   */
  #include 
-__KERNEL_RCSID(0, "$NetBSD: fdt_powerdomain.c,v 1.1 2022/03/04
08:19:06 skrll Exp $");
+__KERNEL_RCSID(0, "$NetBSD: fdt_powerdomain.c,v 1.2 2024/06/12
06:39:28 rin Exp $");
  #include 
@@ -103,7 +103,7 @@ fdtbus_powerdomain_enable_internal(int p
  if (pds == NULL)
  return EINVAL;
-    for (const uint32_t *pd = pds; pd < pds + len; index--) {
+    for (const uint32_t *pd = pds; pd < pds + len / sizeof(*pd);
index--) {
  uint32_t pd_node =
 fdtbus_get_phandle_from_native(be32toh(pd[0]));
  struct fdtbus_powerdomain_controller *pdc =

Re: CVS commit: src/sys/dev/fdt

[Rin Okuyama]

Hmm, there was a confusion for my side.

This bug affected cases where

(1) index >= 1 is explicitly specified for
fdtbus_powerdomain_enable_index(),

as well as

(2) all indices are implicitly specified by
fdtbus_powerdomain_enable().

s/enable/disable/ functions were affected also.

Thanks,
rin

On 2024/06/12 15:39, Rin Okuyama wrote:

Module Name:src
Committed By:   rin
Date:   Wed Jun 12 06:39:28 UTC 2024

Modified Files:
src/sys/dev/fdt: fdt_powerdomain.c

Log Message:
fdt_powerdomain: Fix bug by which pd index >= 1 couldn't be enabled

Length in bytes was mistakenly used as number of uint32_t variables.


To generate a diff of this commit:
cvs rdiff -u -r1.1 -r1.2 src/sys/dev/fdt/fdt_powerdomain.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.


Modified files:

Index: src/sys/dev/fdt/fdt_powerdomain.c
diff -u src/sys/dev/fdt/fdt_powerdomain.c:1.1 
src/sys/dev/fdt/fdt_powerdomain.c:1.2
--- src/sys/dev/fdt/fdt_powerdomain.c:1.1   Fri Mar  4 08:19:06 2022
+++ src/sys/dev/fdt/fdt_powerdomain.c   Wed Jun 12 06:39:28 2024
@@ -1,4 +1,4 @@
-/* $NetBSD: fdt_powerdomain.c,v 1.1 2022/03/04 08:19:06 skrll Exp $ */
+/* $NetBSD: fdt_powerdomain.c,v 1.2 2024/06/12 06:39:28 rin Exp $ */
  
  /*-

   * Copyright (c) 2022 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
   */
  
  #include 

-__KERNEL_RCSID(0, "$NetBSD: fdt_powerdomain.c,v 1.1 2022/03/04 08:19:06 skrll Exp 
$");
+__KERNEL_RCSID(0, "$NetBSD: fdt_powerdomain.c,v 1.2 2024/06/12 06:39:28 rin Exp 
$");
  
  #include 
  
@@ -103,7 +103,7 @@ fdtbus_powerdomain_enable_internal(int p

if (pds == NULL)
return EINVAL;
  
-	for (const uint32_t *pd = pds; pd < pds + len; index--) {

+   for (const uint32_t *pd = pds; pd < pds + len / sizeof(*pd); index--) {
uint32_t pd_node =
   fdtbus_get_phandle_from_native(be32toh(pd[0]));
struct fdtbus_powerdomain_controller *pdc =

Re: CVS commit: src/lib/libc

[Taylor R Campbell]

> Date: Sat, 8 Jun 2024 11:51:43 +0200
> From: Roland Illig 
> 
> Am 07.06.2024 um 22:50 schrieb Taylor R Campbell:
> > libc: Pacify lint on aarch64.
> >
> > +++ src/lib/libc/stdlib/Makefile.incFri Jun  7 20:50:13 2024
> > +# lint(1) spuriously complains about `*s == CHAR_MAX' even though *s
> > +# has type char.
> > +LINTFLAGS.strfmon.c += -X 230
> 
> I guess the "spuriously" here means "on platforms where 'char' is
> unsigned", not "sometimes on the same platform, unpredictably".

Correct.

> Lint does not warn if the constant is defined as a character constant,
> so '\xff' instead of 0xff would work, but I don't know what else would
> be affected if the definition of CHAR_MAX were changed.

Might work but I'm reluctant to try without extensive testing on a lot
of compilers in a lot of environments.  E.g., does that work in C89?

> Ideally, lint would not warn about this expression, but since lint only
> looks at the preprocessed translation unit, it cannot know that the 0xff
> comes from CHAR_MAX and thus is fine.  Practically, suppressing the
> warning in this particular case makes sense.
> 
> Any ideas how to resolve this situation?

No brilliant ideas, sorry, other than to teach lint to track
provenance of constants through macro expansion.  That's why I just
disabled the warning for this case.

Re: CVS commit: src/distrib/sets/lists

[Rin Okuyama]

On 2024/06/04 7:41, Christos Zoulas wrote:

In article <20240603184723.c745cf...@cvs.netbsd.org>,
Taylor R Campbell  wrote:

-=-=-=-=-=-

Module Name:src
Committed By:   riastradh
Date:   Mon Jun  3 18:47:23 UTC 2024

Modified Files:
src/distrib/sets/lists/base32: ad.aarch64 md.sparc64
src/distrib/sets/lists/comp: ad.aarch64
src/distrib/sets/lists/debug32: ad.aarch64 ad.mips64eb ad.mips64el
ad.mipsn64eb ad.mipsn64el ad.powerpc64 ad.riscv64 md.amd64
md.sparc64
src/distrib/sets/lists/debug64: ad.mips64eb ad.mips64el

Log Message:
Take a whack at fixing the compat32 set lists for clang build.

This really needs to be automatically derived from the main set
lists; all this copypasta is a maintenance nightmare and I doubt I
got everything after an hour of tweaking.


Yes, this all needs to be automated or reverted. It is a waste of
time and error-prone to manually have to edit what can be automatically
generated. The clang builds have been broken for more than a month,
and I did not bother to fix them because it is a waste of manual labor.
Then Taylor wasted his time fixing them. Is there a due date by which
all should be fixed or reverted? I propose July 1st.

christos



Yes, this is purely wast of time. This also makes important tasks,
e.g., switching to GCC12, needlessly harder.

I feel that July 1st is too loose deadline, but it should still be
better than nothing...

Thanks,
rin

Re: CVS commit: src/lib/libc

[Roland Illig]

Am 07.06.2024 um 22:50 schrieb Taylor R Campbell:
> libc: Pacify lint on aarch64.
>
> +++ src/lib/libc/stdlib/Makefile.inc  Fri Jun  7 20:50:13 2024
> +# lint(1) spuriously complains about `*s == CHAR_MAX' even though *s
> +# has type char.
> +LINTFLAGS.strfmon.c += -X 230

I guess the "spuriously" here means "on platforms where 'char' is
unsigned", not "sometimes on the same platform, unpredictably".

When CHAR_MAX is defined as 0xff, lint assumes that the same constant
with the same value is used on other platforms as well, thus the
warning.  This is due to lint's -p flag for portability checks.

Lint does not warn if the constant is defined as a character constant,
so '\xff' instead of 0xff would work, but I don't know what else would
be affected if the definition of CHAR_MAX were changed.

Ideally, lint would not warn about this expression, but since lint only
looks at the preprocessed translation unit, it cannot know that the 0xff
comes from CHAR_MAX and thus is fine.  Practically, suppressing the
warning in this particular case makes sense.

Any ideas how to resolve this situation?

Roland

Re: CVS commit: src/distrib/sets/lists

[Christos Zoulas]

In article <20240603184723.c745cf...@cvs.netbsd.org>,
Taylor R Campbell  wrote:
>-=-=-=-=-=-
>
>Module Name:   src
>Committed By:  riastradh
>Date:  Mon Jun  3 18:47:23 UTC 2024
>
>Modified Files:
>   src/distrib/sets/lists/base32: ad.aarch64 md.sparc64
>   src/distrib/sets/lists/comp: ad.aarch64
>   src/distrib/sets/lists/debug32: ad.aarch64 ad.mips64eb ad.mips64el
>   ad.mipsn64eb ad.mipsn64el ad.powerpc64 ad.riscv64 md.amd64
>   md.sparc64
>   src/distrib/sets/lists/debug64: ad.mips64eb ad.mips64el
>
>Log Message:
>Take a whack at fixing the compat32 set lists for clang build.
>
>This really needs to be automatically derived from the main set
>lists; all this copypasta is a maintenance nightmare and I doubt I
>got everything after an hour of tweaking.

Yes, this all needs to be automated or reverted. It is a waste of
time and error-prone to manually have to edit what can be automatically
generated. The clang builds have been broken for more than a month,
and I did not bother to fix them because it is a waste of manual labor.
Then Taylor wasted his time fixing them. Is there a due date by which
all should be fixed or reverted? I propose July 1st.

christos

Re: CVS commit: src/sys/sys

[Ryo ONODERA]

Hi,

Thanks for your fix.
It works fine for me.

"Taylor R Campbell"  writes:

> Module Name:  src
> Committed By: riastradh
> Date: Sat May 25 13:44:48 UTC 2024
>
> Modified Files:
>   src/sys/sys: ucontext.h
>
> Log Message:
> ucontext.h: Expose __UCONTEXT_SIZE to userland.
>
> But don't expose __CTASSERT(__UCONTEXT_SIZE == sizeof(ucontext_t)) to
> userland.
>
> - __UCONTEXT_SIZE will be needed soon by libc and signal trampolines
>   in order to version ucontext so we can safely expand it with
>   architecture extensions.
>
> - __CTASSERT is not safe to use in header files in arbitrary
>   compilation environments (although in the NetBSD kernel build it is
>   safe).  Exposing the __CTASSERT in a header file used by userland
>   appears to have the side effect of breaking the Firefox build in
>   rustc, though the mechanism isn't entirely clear.
>
> Firefox/rustc discussion here:
> https://mail-index.netbsd.org/pkgsrc-users/2024/05/18/msg039578.html
>
> Let's see if this makes a difference -- if it works, great; if not,
> maybe it will help us to narrow down what's happening with rustc and
> Firefox.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.23 -r1.24 src/sys/sys/ucontext.h
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
> Modified files:
>
> Index: src/sys/sys/ucontext.h
> diff -u src/sys/sys/ucontext.h:1.23 src/sys/sys/ucontext.h:1.24
> --- src/sys/sys/ucontext.h:1.23   Fri May 24 13:50:30 2024
> +++ src/sys/sys/ucontext.hSat May 25 13:44:48 2024
> @@ -1,4 +1,4 @@
> -/*   $NetBSD: ucontext.h,v 1.23 2024/05/24 13:50:30 ryoon Exp $  */
> +/*   $NetBSD: ucontext.h,v 1.24 2024/05/25 13:44:48 riastradh Exp $  */
>  
>  /*-
>   * Copyright (c) 1999, 2003, 2024 The NetBSD Foundation, Inc.
> @@ -82,6 +82,10 @@ struct __ucontext {
>  #define _UC_UCONTEXT_ALIGN (~0)
>  #endif
>  
> +#ifndef __UCONTEXT_SIZE
> +#define  __UCONTEXT_SIZE sizeof(ucontext_t)
> +#endif
> +
>  #ifndef _UC_TLSBASE
>  #error   _UC_TLSBASE not defined.
>  #endif
> @@ -105,8 +109,6 @@ int   cpu_mcontext_validate(struct lwp *, 
>  
>  #ifdef __UCONTEXT_SIZE
>  __CTASSERT(sizeof(ucontext_t) == __UCONTEXT_SIZE);
> -#else
> -#define  __UCONTEXT_SIZE sizeof(ucontext_t)
>  #endif
>  #endif /* _KERNEL */
>  
>

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/usr.bin/gzip

[Izumi Tsutsui]

> Your diff is backwards and I guess that is the cause for the confusion.

Ah, I checked a wrong tree. Sorry for the noise.
---
Izumi Tsutsui

Re: CVS commit: src/usr.bin/gzip

[Martin Husemann]

On Sat, Jun 01, 2024 at 11:17:11PM +0900, Izumi Tsutsui wrote:
> > Log Message:
> > Fix typo in previous
> 
> ---
> @@ -1126,7 +1126,7 @@ copymodes(int fd, const struct stat *sbp
>   if (fchmod(fd, sb.st_mode) < 0)
>   maybe_warn("couldn't fchmod: %s", file);
>  
> -#if !HAVE_NBTOOL_CONFIG_H
> +#ifdef !HAVE_NBTOOL_CONFIG_H
[..]
> If HAVE_NBTOOL_CONFIG_H is not defined, it's treated as 0 so
> !HAVE_NBTOOL_CONFIG_H still works.

Your diff is backwards and I guess that is the cause for the confusion.

The change was from #ifdef (which does not work with !) to #if, to
unbreak the build.

The !HAVE_NBTOOL_CONFIG_H is still there and everything should work as
you intended.

Martin

Re: CVS commit: src/usr.bin/gzip

[Izumi Tsutsui]

martin@ wrote:

> Module Name:  src
> Committed By: martin
> Date: Sat Jun  1 10:17:12 UTC 2024
> 
> Modified Files:
>   src/usr.bin/gzip: gzip.c
> 
> Log Message:
> Fix typo in previous

---
@@ -1126,7 +1126,7 @@ copymodes(int fd, const struct stat *sbp
if (fchmod(fd, sb.st_mode) < 0)
maybe_warn("couldn't fchmod: %s", file);
 
-#if !HAVE_NBTOOL_CONFIG_H
+#ifdef !HAVE_NBTOOL_CONFIG_H
TIMESPEC_TO_TIMEVAL(×[0], &sb.st_atimespec);
TIMESPEC_TO_TIMEVAL(×[1], &sb.st_mtimespec);
if (futimes(fd, times) < 0)
---

What's the problem in previous?
gzip.c already has the following lines:

>> #if HAVE_NBTOOL_CONFIG_H
>> #include "nbtool_config.h"
>> #endif

and  also has
>> #if !HAVE_NBTOOL_CONFIG_H
>> #define DISKUNIT(dev)   (minor(dev) / MAXPARTITIONS)
>> #define DISKPART(dev)   (minor(dev) % MAXPARTITIONS)
>> #define DISKMINOR(unit, part) \
>> (((unit) * MAXPARTITIONS) + (part))
>> #endif /* !HAVE_NBTOOL_CONFIG_H */
(though  etc uses !defined(HAVE_NBTOOL_CONFIG_H))

If HAVE_NBTOOL_CONFIG_H is not defined, it's treated as 0 so
!HAVE_NBTOOL_CONFIG_H still works.

If you don't like !HAVE_NBTOOL_CONFIG_H, could you please fix
all other files?

Thanks,
---
Izumi Tsutsui

Re: CVS commit: src/sys

[Jason Thorpe]

What issue is this change attempting to resolve?

> On May 20, 2024, at 4:34 AM, Taylor R Campbell  wrote:
> 
> Module Name: src
> Committed By: riastradh
> Date: Mon May 20 11:34:19 UTC 2024
> 
> Modified Files:
> src/sys/arch/sparc64/dev: pci_machdep.c
> src/sys/arch/sparc64/include: pci_machdep.h
> src/sys/arch/xen/include: pci_machdep.h
> src/sys/arch/xen/xen: xpci_xenbus.c
> src/sys/dev/acpi: acpi_mcfg.c
> src/sys/dev/pci: pci.c pcivar.h
> 
> Log Message:
> pci: Pass cookie through pci_find_device, pci_enumerate_bus.
> 
> New functions pci_find_device1 and pci_enumerate_bus1 have the cookie
> argument.  Existing symbols pci_find_device and pci_enumerate_bus are
> now wrappers for the cookieless version.
> 
> This drops the symbol pci_probe_device, in favour of a new
> pci_probe_device1 with the cookie argument.  But I don't think that
> requires a revbump because it's only called by MD pci_enumerate_bus1
> implementations, which don't live in modules anyway.
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.80 -r1.81 src/sys/arch/sparc64/dev/pci_machdep.c
> cvs rdiff -u -r1.28 -r1.29 src/sys/arch/sparc64/include/pci_machdep.h
> cvs rdiff -u -r1.21 -r1.22 src/sys/arch/xen/include/pci_machdep.h
> cvs rdiff -u -r1.26 -r1.27 src/sys/arch/xen/xen/xpci_xenbus.c
> cvs rdiff -u -r1.26 -r1.27 src/sys/dev/acpi/acpi_mcfg.c
> cvs rdiff -u -r1.165 -r1.166 src/sys/dev/pci/pci.c
> cvs rdiff -u -r1.117 -r1.118 src/sys/dev/pci/pcivar.h
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
> 

-- thorpej

Re: CVS commit: src/usr.bin/make

[Simon J. Gerraty]

Taylor R Campbell  wrote:
> Can you please back this out promptly, add automatic tests for
> whatever the underlying issue is, and redo it another way?

I did a scan of miss-use of <>, and looks like libc might
have an issue:

lib/libc/arch/arm/Makefile.inc:28:.include 

expects to find lib/libc/softfloat/Makefile.inc

via .CURDIR, so have re-enabled that search.

re: CVS commit: src

[matthew green]

thanks for this fix.

> Modified Files:
>   src/external/gpl3/gcc/dist/libstdc++-v3/config/io: basic_file_stdio.cc

probably want to apply this fix to gcc.old, and also consider
pullup-10 (and pullup-9, if similar?), since gcc.old is the
currently used compiler in current.


.mrg.

Re: CVS commit: src/usr.bin/make

[Simon J. Gerraty]

Taylor R Campbell  wrote:
> 
> --- cleandir-libterminfo ---
> nbmake[5]: "/tmp/build/2024.05.19.20.09.40-i386/src/lib/libterminfo/Makefile" 
> line 50: Could not find Makefile.hash
> 
> Can you please back this out promptly, add automatic tests for
> whatever the underlying issue is, and redo it another way?

Sorry, for the break.
The makefile in lib/libterminfo was using <> when it should use "".
Martin already fixed it.

I'm checking for similar 

Re: CVS commit: src/usr.bin/make

[Taylor R Campbell]

> Module Name:src
> Committed By:   sjg
> Date:   Sun May 19 20:09:40 UTC 2024
> 
> Modified Files:
> src/usr.bin/make: dir.c dir.h parse.c
> 
> Log Message:
> make: use separate function to include makefiles.
> 
> Have Dir_FindFile and Dir_FindInclude call FindFile with a
> bool flag to indicate whether .CURDIR should be be searched at all.

This appears to have broken all of the builds:

https://releng.netbsd.org/builds/HEAD/202405200920Z/
https://releng.netbsd.org/b5reports/i386/commits-2024.05.html#2024.05.19.20.09.40
https://releng.netbsd.org/b5reports/i386/2024/2024.05.19.20.09.40/build.log.tail

--- cleandir-libterminfo ---
nbmake[5]: "/tmp/build/2024.05.19.20.09.40-i386/src/lib/libterminfo/Makefile" 
line 50: Could not find Makefile.hash

Can you please back this out promptly, add automatic tests for
whatever the underlying issue is, and redo it another way?

Re: CVS commit: src

[Jason Thorpe]

> On May 19, 2024, at 6:21 PM, Christos Zoulas  wrote:
> 
> We discussed it with Taylor and decided to call it dup3100 after all since 
> we've
> already followed the old scheme for kevent100 and it is better to be 
> consistent
> for the 11 release.We can start calling new syscalls 12 after 11 is released.

But how can the old one be in compat_11?

-- thorpej

Re: CVS commit: src

[Christos Zoulas]

> On May 19, 2024, at 9:19 PM, Jason Thorpe  wrote:
> 
> 
>> On May 19, 2024, at 5:35 PM, Christos Zoulas  wrote:
>> 
>> In article <9c72736d-707b-4267-bd05-45bffea52...@me.com>,
>> Jason Thorpe   wrote:
>>> 
>>> 
 On May 19, 2024, at 3:25 PM, Christos Zoulas  wrote:
 
 src/sys/compat/common: compat_110_mod.c sys_decrip_110.c
 src/sys/compat/netbsd32: netbsd32_compat_110.c
 src/sys/conf: compat_netbsd110.config
 src/sys/modules/compat_110: Makefile
 src/sys/modules/compat_netbsd32_110: Makefile
>>> 
>>> Wait, why is there now a compat_110 module?  netbsd-11 isn’t out yet. 
>>> dup3() belongs in the compat_100 module.
>> 
>> You asked for the syscall to be called dup3110 so I put it in compat_110...
> 
> The new system call is dup3110 (“dup3 for 11.0”), the old one is “netbsd-10 
> compatibility”, so belongs in compat_10.
> 

We discussed it with Taylor and decided to call it dup3100 after all since we've
already followed the old scheme for kevent100 and it is better to be consistent
for the 11 release.We can start calling new syscalls 12 after 11 is released.

christos

Re: CVS commit: src

[Jason Thorpe]

> On May 19, 2024, at 5:35 PM, Christos Zoulas  wrote:
> 
> In article <9c72736d-707b-4267-bd05-45bffea52...@me.com>,
> Jason Thorpe   wrote:
>> 
>> 
>>> On May 19, 2024, at 3:25 PM, Christos Zoulas  wrote:
>>> 
>>> src/sys/compat/common: compat_110_mod.c sys_decrip_110.c
>>> src/sys/compat/netbsd32: netbsd32_compat_110.c
>>> src/sys/conf: compat_netbsd110.config
>>> src/sys/modules/compat_110: Makefile
>>> src/sys/modules/compat_netbsd32_110: Makefile
>> 
>> Wait, why is there now a compat_110 module?  netbsd-11 isn’t out yet. 
>> dup3() belongs in the compat_100 module.
> 
> You asked for the syscall to be called dup3110 so I put it in compat_110...

The new system call is dup3110 (“dup3 for 11.0”), the old one is “netbsd-10 
compatibility”, so belongs in compat_10.

-- thorpej

Re: CVS commit: src

[Christos Zoulas]

In article <9c72736d-707b-4267-bd05-45bffea52...@me.com>,
Jason Thorpe   wrote:
>
>
>> On May 19, 2024, at 3:25 PM, Christos Zoulas  wrote:
>> 
>> src/sys/compat/common: compat_110_mod.c sys_decrip_110.c
>> src/sys/compat/netbsd32: netbsd32_compat_110.c
>> src/sys/conf: compat_netbsd110.config
>> src/sys/modules/compat_110: Makefile
>> src/sys/modules/compat_netbsd32_110: Makefile
>
>Wait, why is there now a compat_110 module?  netbsd-11 isn’t out yet. 
>dup3() belongs in the compat_100 module.

You asked for the syscall to be called dup3110 so I put it in compat_110...

christos

Re: CVS commit: src

[Jason Thorpe]

> On May 19, 2024, at 3:25 PM, Christos Zoulas  wrote:
> 
> src/sys/compat/common: compat_110_mod.c sys_decrip_110.c
> src/sys/compat/netbsd32: netbsd32_compat_110.c
> src/sys/conf: compat_netbsd110.config
> src/sys/modules/compat_110: Makefile
> src/sys/modules/compat_netbsd32_110: Makefile

Wait, why is there now a compat_110 module?  netbsd-11 isn’t out yet.  dup3() 
belongs in the compat_100 module.

-- thorpej

Re: CVS commit: src/share/man/man4

[nia]

On Thu, May 16, 2024 at 05:48:05PM +0300, Valery Ushakov wrote:
> On Thu, May 16, 2024 at 11:54:20 +, Nia Alarie wrote:
> 
> > Modified Files:
> > src/share/man/man4: eap.4
> > 
> > Log Message:
> > Note that EAP_USE_BOTH_DACS is deprecated in the eap(4) manual page.
> 
> Please, can you restore the part that explains what this option
> is/does?  It might be on its way out, but since we document it's
> there, it's a good idea to actually document it, IMHO.
> 
> I don't know much about audio, but the kernel mixer is software, isn't
> it.  I would imagine the type of systems that might have this device
> may actually benefit from the hardware acceleration that this option
> seems to imply.
> 
> I.e. if anything, I'd rather this option is documented even better
> than it was.

Let me know if the new version of the text leaves any doubts.

Re: CVS commit: src/share/man/man4

[Valery Ushakov]

On Thu, May 16, 2024 at 11:54:20 +, Nia Alarie wrote:

> Modified Files:
>   src/share/man/man4: eap.4
> 
> Log Message:
> Note that EAP_USE_BOTH_DACS is deprecated in the eap(4) manual page.

Please, can you restore the part that explains what this option
is/does?  It might be on its way out, but since we document it's
there, it's a good idea to actually document it, IMHO.

I don't know much about audio, but the kernel mixer is software, isn't
it.  I would imagine the type of systems that might have this device
may actually benefit from the hardware acceleration that this option
seems to imply.

I.e. if anything, I'd rather this option is documented even better
than it was.

-uwe

Re: CVS commit: src/bin/echo

[Robert Elz]

Date:Wed, 15 May 2024 02:33:23 +0300
From:Valery Ushakov 
Message-ID:  

  | I vaguely remember I read somewhere that printf(1) was specifically
  | conceived to take no options, but that can be planted memories.  May
  | be it's indeed induced by the old state of affairs in our version.

POSIX printf(1) has no options (it says in its page "OPTIONS: None")
and our printf has no options, but implementations are allowed to
extend the standard, and add some, POSIX says (XCU 1.4) that when it
says "OPTIONS: None" what it means is:

  Default Behavior: When this section is listed as ``None.'', it means that
  the implementation need not support any options. Standard utilities that
  do not accept options, but that do accept operands, shall recognize "--"
  as a first argument to be discarded.

That is more or less what we do, but for backwards compat reasons, not
always.

  The requirement for recognizing "--" is because conforming applications
  need a way to shield their operands from any arbitrary options that the
  implementation may provide as an extension. For example, if the standard
  utility foo is listed as taking no options, and the application needed
  to give it a pathname with a leading , it could safely do
  it as:

foo -- -myfile

  and avoid any problems with -m used as an extension.

Other versions of printf(1) do have options, or at least, an option.
bash, ksh93 & zsh all have a "-v var" option to cause the output from
(the builtin) printf to be stored in var, rather than written to stdout.

It might be time to adjust our ptintf to be fully POSIX, and always handle
the "--" end of options option (it doesn't seem useful as a printf format,
and if needed, can always be written as printf -- --) rather than only
doing it sometimes.   Currently our printf used as "printf --" writes "--"
to stdout.   It really shouldn't.   When there is just 1 arg, and it
contains no %, it is also treated as a format, even if it starts with a '-'.

So, as you demonstrated, in our printf, printf -V prints "-V", but
if you try 'printf -V A B C' what you get is:

  sh: unknown option -- V
  Usage: printf format [arg ...]

(It would do the same if given a script with one of the -v var options
used in it).

The comments at the start of main() in our printf(1) source are nonsense
(though for backwards compat, we should just check for "--" rather than
using getopt() to do that for us, as we currently do, when we do it.)

kre

ps: I do appreciate that some of the mess that is there now is my doing,
but back when I did that I didn't understand POSIX as well as I do now.

Re: CVS commit: src/bin/echo

[Valery Ushakov]

On Wed, May 15, 2024 at 05:22:25 +0700, Robert Elz wrote:

>   | Unfortunately that advice is not true without further caveats.
> 
> That you have to actually write a valid printf(1) command, and not
> simply s/echo/printf/ ?   Does that really need saying?
> 
> 
>   | netbsd$ sh -c "printf '-V\n'"
> 
>   printf -- -V\\n 
> 
> and it will work anywhere - our printf is specially hacked as once
> upon a time it took no options, and this kind of thing would work.
> Format strings starting with a '-' don't work in general however,
> the '--' should be included if the format might begin with a '-'.
> 
> Even better would be
> 
>   printf -- %s\\n -V
> 
> (where the -- is optional here).

I vaguely remember I read somewhere that printf(1) was specifically
conceived to take no options, but that can be planted memories.  May
be it's indeed induced by the old state of affairs in our version.


-uwe

Re: CVS commit: src/bin/echo

[Robert Elz]

Date:Tue, 14 May 2024 12:41:51 +0300
From:Valery Ushakov 
Message-ID:  

  | Unfortunately that advice is not true without further caveats.

That you have to actually write a valid printf(1) command, and not
simply s/echo/printf/ ?   Does that really need saying?


  | netbsd$ sh -c "printf '-V\n'"

printf -- -V\\n 

and it will work anywhere - our printf is specially hacked as once
upon a time it took no options, and this kind of thing would work.
Format strings starting with a '-' don't work in general however,
the '--' should be included if the format might begin with a '-'.

Even better would be

printf -- %s\\n -V

(where the -- is optional here).

kre

aside: I'd use '-V\n' inside "" as well.   But as a sh -c command
string I'd use:

sh -c 'printf -- -V\\n'

Re: CVS commit: src/bin/echo

[Valery Ushakov]

On Tue, May 14, 2024 at 01:32:25 +, David H. Gutteridge wrote:

> Log Message:
> echo.1: borrow advice about printf(1) from the OpenBSD man page

Unfortunately that advice is not true without further caveats.

netbsd$ sh -c "printf '-V\n'"
-V

$ busybox sh -c "printf '-V\n'"
-V

ubuntu$ $ dash -c "printf '-V\n'"
dash: 1: printf: Illegal option -V

$ bash -c "printf '-V\n'"
bash: line 1: printf: -V: invalid option
printf: usage: printf [-v var] format [arguments]


-uwe

lint's strict bool mode (was: Re: CVS commit: src/usr.sbin/flashctl)

[Roland Illig]

Am 13.05.2024 um 16:06 schrieb Taylor R Campbell:
>> Module Name:src
>> Committed By:   rillig
>> Date:   Sun May 12 19:03:55 UTC 2024
>>
>> Modified Files:
>> src/usr.sbin/flashctl: flashctl.c
>>
>> Log Message:
>> flashctl: fix lint's strict bool mode with Clang preprocessor
>>
>> Treating the return value from the  character classification
>> functions as an 'int' is neither elegant nor idiomatic, but it works for
>> now.
>>
>> -   if (!isxdigit((unsigned char)str[2]))
>> +   if (isxdigit((unsigned char)str[2]) == 0)
>
> Why is this change necessary?  Weren't you teaching lint to handle
> this case without complaining?

Yes, I did. The thing is that I only ever tested lint's strict bool mode
with the GCC preprocessor, and that preprocessor marks every token from
a macro expansion as coming from a system header or coming from the
user's code. Therefore, lint could be more lenient when checking the
result of the isxdigit macro, as its main operator, the cast to '(int)',
is from a system header, so it's OK that it has the "wrong" type.

On the other hand, the strcmp function is not listed in "namespace.h"
and does not have a macro definition, and although the function
declaration comes from a system header, the GCC preprocessor does not
mark the function call expression as coming from a system header.

This is the difference by which lint treats 'isxdigit(ch)' as bool-like
but 'strcmp(s1, s2)' as strictly 'int'.

A few days or weeks ago, Christos started builds that have both
MKLLVM=yes and MKLINT=yes, thus using the Clang preprocessor as lint's
preprocessor. Apparently nobody else has done this in the last few
years. Clang's preprocessor's output does not mark the tokens from macro
expansions as coming from a system header or from the user's code. Due
to this, lint can no longer be lenient for system header macro
expansions as it doesn't get the system header information anymore.

> We shouldn't change anything like
>
>   if (!isxdigit(...))
>   if (ferror(...))
>
> to
>
>   if (isxdigit(...) == 0)
>   if (ferror(...) != 0)
>
> The original is clearer and idiomatic code, even if it's a little
> silly that the return value is declared as int and not bool
> (presumably for historical reasons, if the interfaces were defined
> before bool existed).

I agree. For usr.bin/error, I tried a different variant, namely to only
activate lint's strict bool mode when MKLLVM is undefined, thus when the
GCC preprocessor is used. I guess activating the strict bool mode only
in GCC mode is good enough to catch all type mismatches.

The combination of MKLLVM=yes MKLINT=yes is also the cause why lint now
allows do-while-0 even in strict bool mode, as the FD_ZERO macro uses
that idiom and I didn't dare to change the macro to do-while-false, as
that would either require , or to deviate from the idiom
by using 'do { ... } while (0 != 0)', as that would look suspicious.

I will switch usr.sbin/flashctl to the only-in-GCC-mode variant and
restore the previous idiomatic code.

Roland

Re: CVS commit: src/usr.sbin/flashctl

[Taylor R Campbell]

> Module Name:src
> Committed By:   rillig
> Date:   Sun May 12 19:03:55 UTC 2024
> 
> Modified Files:
> src/usr.sbin/flashctl: flashctl.c
> 
> Log Message:
> flashctl: fix lint's strict bool mode with Clang preprocessor
> 
> Treating the return value from the  character classification
> functions as an 'int' is neither elegant nor idiomatic, but it works for
> now.
> 
> -   if (!isxdigit((unsigned char)str[2]))
> +   if (isxdigit((unsigned char)str[2]) == 0)

Why is this change necessary?  Weren't you teaching lint to handle
this case without complaining?  We shouldn't change anything like

if (!isxdigit(...))
if (ferror(...))

to

if (isxdigit(...) == 0)
if (ferror(...) != 0)

The original is clearer and idiomatic code, even if it's a little
silly that the return value is declared as int and not bool
(presumably for historical reasons, if the interfaces were defined
before bool existed).

Re: CVS commit: src/distrib/common

[Robert Elz]

Date:Thu, 9 May 2024 17:44:16 -0400
From:Christos Zoulas 
Message-ID:  <9c618434-9d7b-4f1c-97ed-3260b7f36...@zoulas.com>

  | I am not sure either but the resulting cd does not boot anymore.

Which version?   It has been a long time since I needed to boot
from an optical drive, but I could try it.

Its a little hard to believe that the uid/gid settings could make
any difference, though the m9de settings might.

kre

Re: CVS commit: src/distrib/common

[Christos Zoulas]

> On May 9, 2024, at 5:37 PM, Robert Elz  wrote:
> 
> Date:Thu, 9 May 2024 12:09:03 -0400
>From:"Christos Zoulas" 
>Message-ID:  <20240509160903.6e41bf...@cvs.netbsd.org>
> 
>  | Instead of augmenting the platform spec with an autogenerated one,
>  | we should understand why we have missing entries in the first place.
> 
> Is it really important?   Everything was fine without it previously,
> except that reproducible builds, weren't (as local info was being
> used in the CD images).
> 
> As long as the "augmenting" is just providing info for files that
> weren't otherwise specified (an issue I admit to not checking in the
> change I made) - that is, as long as makefs takes the first entry in
> the spec file it is given, should there be more than one for a file
> (if it prefers to use the last, then that would just change the order
> in which the augmentation file is included, no need to change makefs)
> then I'm not really sure what the problem is.

I am not sure either but the resulting cd does not boot anymore. I am trying to 
understand why. If augmenting works as you describe, there is more problem...

christos
> 
> That is, I don't see we need to add yet another manual maintenance step
> that we need to make in order to add or remove a file from a CD image
> (or whatever the image is destined for) when the information being added
> isn't really important - but just needs to be consistent.
> 
> kre

Re: CVS commit: src/distrib/common

[Robert Elz]

Date:Thu, 9 May 2024 12:09:03 -0400
From:"Christos Zoulas" 
Message-ID:  <20240509160903.6e41bf...@cvs.netbsd.org>

  | Instead of augmenting the platform spec with an autogenerated one,
  | we should understand why we have missing entries in the first place.

Is it really important?   Everything was fine without it previously,
except that reproducible builds, weren't (as local info was being
used in the CD images).

As long as the "augmenting" is just providing info for files that
weren't otherwise specified (an issue I admit to not checking in the
change I made) - that is, as long as makefs takes the first entry in
the spec file it is given, should there be more than one for a file
(if it prefers to use the last, then that would just change the order
in which the augmentation file is included, no need to change makefs)
then I'm not really sure what the problem is.

That is, I don't see we need to add yet another manual maintenance step
that we need to make in order to add or remove a file from a CD image
(or whatever the image is destined for) when the information being added
isn't really important - but just needs to be consistent.

kre

re: CVS commit: src/external/mit/xorg/lib

[matthew green]

"Taylor R Campbell" writes:
> Module Name:  src
> Committed By: riastradh
> Date: Sun May  5 15:25:31 UTC 2024
>
> Modified Files:
>   src/external/mit/xorg/lib/dri: Makefile
>   src/external/mit/xorg/lib/dri.old: Makefile
>
> Log Message:
> mesa: Build with -Wno-error=typedef-redefinition.
>
> While here, use CWARNFLAGS.clang instead of an explicit conditional.
>
> In file included from 110_blorp_exec.c:33:
> In file included from 
> /home/source/ab/HEAD-llvm/xsrc/external/mit/MesaLib/dist/src/intel/blorp/blorp_genX_exec.h:27:
> In file included from 
> /home/source/ab/HEAD-llvm/xsrc/external/mit/MesaLib/dist/src/intel/blorp/blorp_priv.h:30:
> /home/source/ab/HEAD-llvm/xsrc/external/mit/MesaLib/dist/src/compiler/nir/nir.h:3840:3:
>  error: redefinition of typedef 'nir_shader' is a C11 feature [-Werror
> ,-Wtypedef-redefinition]
> } nir_shader;
>   ^
> /home/source/ab/HEAD-llvm/xsrc/external/mit/MesaLib/dist/src/intel/compiler/brw_compiler.h:41:27:
>  note: previous definition is here
> typedef struct nir_shader nir_shader;

actually, this really is a valid warning and the real fix is to not
compile this with -std=gnu99 (or is it gnu+99), but use the default
for some and maybe gnu++17 for some parts, going on what the real
build for this does.

not sure why you changed dri.old/Makefile.  it wasn't a problem.

i was testing the real fix and i'll revert these when it's in.


.mrg.

Re: Clang vs. lint's strict bool mode (was: Re: CVS commit: src/usr.bin/base64)

[Christos Zoulas]

> On May 3, 2024, at 11:11 AM, Roland Illig  wrote:
> 
> Am 02.05.2024 um 22:44 schrieb Christos Zoulas:
>> On 2024-05-02 3:04 pm, Christos Zoulas wrote:
>>> This is with clang.
>>> 
>> And I don't get it. Gcc produces:
>> 
>>   if (ignore &&
>> # 139 "base64.c" 3 4
>>((int)((_ctype_tab_ + 1)[(
>> # 139 "base64.c"
>>c
>> # 139 "base64.c" 3 4
>>)] & 0x0040))
>> # 139 "base64.c"
>>  )
>>continue;
> 
> Since the '&&' in the condition comes from a system header, lint is not
> as strict as it could be, to allow user code to pass strict bool mode
> even when the system headers don't comply.
> 
>> Clang produces:
>>   if (ignore && ((int)((_ctype_tab_ + 1)[(c)] & 0x0040)))
>>continue;
> 
> Clang doesn't distinguish the system parts from the non-system parts,
> and that's the crucial difference. I never tested the combination of
> Clang with lint. I will do so now.

Got it, thank you!

christos

Clang vs. lint's strict bool mode (was: Re: CVS commit: src/usr.bin/base64)

[Roland Illig]

Am 02.05.2024 um 22:44 schrieb Christos Zoulas:
> On 2024-05-02 3:04 pm, Christos Zoulas wrote:
>> This is with clang.
>>
> And I don't get it. Gcc produces:
>
>if (ignore &&
> # 139 "base64.c" 3 4
> ((int)((_ctype_tab_ + 1)[(
> # 139 "base64.c"
> c
> # 139 "base64.c" 3 4
> )] & 0x0040))
> # 139 "base64.c"
>   )
> continue;

Since the '&&' in the condition comes from a system header, lint is not
as strict as it could be, to allow user code to pass strict bool mode
even when the system headers don't comply.

> Clang produces:
>if (ignore && ((int)((_ctype_tab_ + 1)[(c)] & 0x0040)))
> continue;

Clang doesn't distinguish the system parts from the non-system parts,
and that's the crucial difference. I never tested the combination of
Clang with lint. I will do so now.

Roland

Re: CVS commit: src/usr.bin/base64

[Christos Zoulas]

On 2024-05-02 3:04 pm, Christos Zoulas wrote:

On 2024-05-02 2:47 pm, Roland Illig wrote:

Am 02.05.2024 um 17:45 schrieb Christos Zoulas:

Module Name:src
Committed By:   christos
Date:   Thu May  2 15:45:36 UTC 2024

Modified Files:
src/usr.bin/base64: Makefile

Log Message:
comment out strict boolean lint check because isspace() returns int 
and lint

complains.


In which exact environment did you experience this?

Lint's strict bool mode accepts 'a & b' as having either integer or
boolean type, so the macro version of isspace should definitely work.

The function variant of isspace doesn't work, though. So maybe you are
running outside _NETBSD_SOURCE mode or you have defined 
_CTYPE_NOINLINE.


Any idea how lint can accept isspace as returning int/bool while not
assuming int/bool for strcmp? One idea is to explicitly list the
"bool-like" functions from the C standard library internally in lint,
another more flexible approach is to have a function attribute
__declared_int_but_actually_bool.


This is with clang.


And I don't get it. Gcc produces:

  if (ignore &&
# 139 "base64.c" 3 4
   ((int)((_ctype_tab_ + 1)[(
# 139 "base64.c"
   c
# 139 "base64.c" 3 4
   )] & 0x0040))
# 139 "base64.c"
 )
   continue;

Clang produces:
  if (ignore && ((int)((_ctype_tab_ + 1)[(c)] & 0x0040)))
   continue;

and lint complains...

christos

Re: CVS commit: src/tests/lib/libm

[Taylor R Campbell]

> Date: Thu, 2 May 2024 21:04:38 +0200
> From: Roland Illig 
> 
> Am 02.05.2024 um 05:30 schrieb Robert Elz:
> > Use intmax_t instead of long int when trying to represent very large
> > integers (10^50 or so), so we don't exceed the capacity of systems where
> > long int is only 32 bits.
> 
> I particularly avoid the types 'long' and 'long double', as they vary
> between the platforms.

In this case, the whole point of the exercise is to test a long double
function nearbyintl distinctly from the double function nearbyint.
The integer result could have been int64_t instead of intmax_t (and
maybe it should be).

I wrote it as long mainly because I copied the nearbyint tests to make
the nearbyintl tests and just forgot to update the long to make it
work on LP32 platforms -- I tested on amd64 before committing and I
was thinking about how sparc64/aarch64 must have broken nearbyintl
(which the test has now confirmed they are).

> Curiously, intmax_t is a 64-bit type even on amd64, where __int128_t is
> also available, but I don't use that because that type is not predefined.

Yes, although intmax_t looks convenient for arithmetic, it was a
mistake to bake it into any ABI like printf "%jd", because it means
expanding intmax_t from 64-bit to 128-bit breaks the ABI.

Had the rule been to use PRIdMAX instead of "%jd" in the C standard,
and had all intmax_t-related functions been defined as macros or
static inlines, this problem could have been avoided.  But it's too
late for that now, so intmax_t is effectively just a confusingly-named
alias for int64_t in practice.

Re: CVS commit: src/usr.bin/base64

[Christos Zoulas]

On 2024-05-02 2:47 pm, Roland Illig wrote:

Am 02.05.2024 um 17:45 schrieb Christos Zoulas:

Module Name:src
Committed By:   christos
Date:   Thu May  2 15:45:36 UTC 2024

Modified Files:
src/usr.bin/base64: Makefile

Log Message:
comment out strict boolean lint check because isspace() returns int 
and lint

complains.


In which exact environment did you experience this?

Lint's strict bool mode accepts 'a & b' as having either integer or
boolean type, so the macro version of isspace should definitely work.

The function variant of isspace doesn't work, though. So maybe you are
running outside _NETBSD_SOURCE mode or you have defined 
_CTYPE_NOINLINE.


Any idea how lint can accept isspace as returning int/bool while not
assuming int/bool for strcmp? One idea is to explicitly list the
"bool-like" functions from the C standard library internally in lint,
another more flexible approach is to have a function attribute
__declared_int_but_actually_bool.


This is with clang.

christos

Re: CVS commit: src/tests/lib/libm

[Roland Illig]

Am 02.05.2024 um 05:30 schrieb Robert Elz:
> Module Name:  src
> Committed By: kre
> Date: Thu May  2 03:30:07 UTC 2024
>
> Modified Files:
>   src/tests/lib/libm: t_fe_round.c
>
> Log Message:
> Use intmax_t instead of long int when trying to represent very large
> integers (10^50 or so), so we don't exceed the capacity of systems where
> long int is only 32 bits.

In the lint tests (which don't have a C preprocessor available), I use
the following typedefs to get fixed-width integer types:

typedef signed char int8_t;
typedef short int16_t;
typedef int int32_t;
typedef long long int64_t;

These types are the same on all platforms supported by lint, and I guess
by NetBSD as a whole.

I particularly avoid the types 'long' and 'long double', as they vary
between the platforms.

Curiously, intmax_t is a 64-bit type even on amd64, where __int128_t is
also available, but I don't use that because that type is not predefined.

Roland

Re: CVS commit: src/usr.bin/base64

[Roland Illig]

Am 02.05.2024 um 17:45 schrieb Christos Zoulas:
> Module Name:  src
> Committed By: christos
> Date: Thu May  2 15:45:36 UTC 2024
>
> Modified Files:
>   src/usr.bin/base64: Makefile
>
> Log Message:
> comment out strict boolean lint check because isspace() returns int and lint
> complains.

In which exact environment did you experience this?

Lint's strict bool mode accepts 'a & b' as having either integer or
boolean type, so the macro version of isspace should definitely work.

The function variant of isspace doesn't work, though. So maybe you are
running outside _NETBSD_SOURCE mode or you have defined _CTYPE_NOINLINE.

Any idea how lint can accept isspace as returning int/bool while not
assuming int/bool for strcmp? One idea is to explicitly list the
"bool-like" functions from the C standard library internally in lint,
another more flexible approach is to have a function attribute
__declared_int_but_actually_bool.

Roland

Re: CVS commit: src/tests/lib/libm

[Robert Elz]

And yes, I know, it should have been 2^50 not 10^50...

kre

Re: CVS commit: src/lib/libutil

[Robert Elz]

Date:Wed, 1 May 2024 22:27:02 +0200
From:Roland Illig 
Message-ID:  <754bd755-be0a-4eff-aa7b-d53fce9b4...@gmx.de>

  | > Log Message:
  | > next should increement by 1 not 2.
  |
  | Are you sure?

I agree, that change should be reverted.

"next" is even documented to be two ...

 6  Despite what is stated above, ?next? is actually 2.  The input ?next
January?, instead of producing a timestamp for January of the
following year, produces one for January 2nd, of the current year.
Use caution with ?next? it rarely does what humans expect.  For
example, on a Sunday ?next sunday? means the following Sunday (7 days
hence) whereas ?next monday? means the monday that follows that (8
days hence) rather than ?tomorrow? or just ?Mon? (without the ?next?)
which is the nearest subsequent Monday.

and is actually more useful that way.   It is peculiar, but that's how it
has worked ~forever.

There are limits to how sane a half hearted (but still useful) attempt
to understand English idiom can possibly work, we really don't want to
attempt to turn parsedate into an AI machine.

kre

Re: CVS commit: src/lib/libutil

[Roland Illig]

Am 01.05.2024 um 21:59 schrieb Christos Zoulas:
> Module Name:  src
> Committed By: christos
> Date: Wed May  1 19:59:08 UTC 2024
>
> Modified Files:
>   src/lib/libutil: parsedate.y
>
> Log Message:
> next should increement by 1 not 2.

Are you sure? I get these test results:

> t_parsedate (2/5): 7 test cases
> atsecs: [0.006548s] Passed.
> dates: [0.007241s] Passed.
> dsttimes: [0.007517s] Passed.
> gibberish: [0.007637s] Passed.
> relative: [0.287375s] Failed: 2258 checks failed; see output for more 
> details
> times: [0.008402s] Passed.
> zones: [0.007586s] Passed.

After your change, "next sunday" goes backward in time, for example:

> From 28110552 (Sun Nov 22 08:29:12 1970) using "next sunday",
> obtained 2808 (Sun Nov 22 00:00:00 1970);
> expected 28684800 (Sun Nov 29 00:00:00 1970)