Re: Diff: Function Length Reduction

[Tom Cosgrove]

For example, making it look something like this would be better

static void
inc_buf(SCR *sp, VICMD *vp)
{
CHAR_T v = vp->buffer;

if (v < '1' || v > '8') return;

VIP(sp)->sdot.buffer = vp->buffer = ++v;
}


Thanks

Tom

On 10/09/2021 22:24, Stuart Henderson wrote:

On 2021/09/10 16:39, VARIK VALEFOR wrote:

Is any particular aspect of the replacement code bad?


The use of 'magic number' ASCII values obfuscates what the code is
doing.

Re: Remove dead assignment in login(1)

[Tom Cosgrove]

>>> Tom Cosgrove  7-Aug-18 20:14 >>>
>
> >>> Frederic Cambus  7-Aug-18 16:15 >>>
> >
> > Hi tech@,
> >
> > Since rev 1.36, the instance variable is never read again so we can
> > simply drop the else clause with the assignment.
> >
> > Comments? OK?
> >
> > Index: usr.bin/login/login.c
> > =
> RCS file: /cvs/src/usr.bin/login/login.c,v
> > retrieving revision 1.69
> > diff -u -p -r1.69 login.c
> > --- usr.bin/login/login.c   8 Dec 2017 17:04:15 -   1.69
> > +++ usr.bin/login/login.c   7 Aug 2018 13:47:00 -
> > @@ -406,8 +406,7 @@ main(int argc, char *argv[])
> > if (strncmp(instance + 1, "root", 4) == 0)
> > rootlogin = 1;
> > *instance++ = '\0';
>
> You should also remove this line ^

Err, no you shouldn't - sorry.  (Note to self: don't email after alcohol, 
especially to lists)

Just ok tom@

> > -   } else
> > -   instance = "";
> > +   }
> >  
> > if (strlen(username) > UT_NAMESIZE)
> > username[UT_NAMESIZE] = '\0';

Re: Remove dead assignment in login(1)

[Tom Cosgrove]

>>> Frederic Cambus  7-Aug-18 16:15 >>>
>
> Hi tech@,
>
> Since rev 1.36, the instance variable is never read again so we can
> simply drop the else clause with the assignment.
>
> Comments? OK?
>
> Index: usr.bin/login/login.c
> ==
> RCS file: /cvs/src/usr.bin/login/login.c,v
> retrieving revision 1.69
> diff -u -p -r1.69 login.c
> --- usr.bin/login/login.c 8 Dec 2017 17:04:15 -   1.69
> +++ usr.bin/login/login.c 7 Aug 2018 13:47:00 -
> @@ -406,8 +406,7 @@ main(int argc, char *argv[])
>   if (strncmp(instance + 1, "root", 4) == 0)
>   rootlogin = 1;
>   *instance++ = '\0';

You should also remove this line ^

With that, ok tom@

> - } else
> - instance = "";
> + }
>  
>   if (strlen(username) > UT_NAMESIZE)
>   username[UT_NAMESIZE] = '\0';

Re: signed overflow in atan2

[Tom Cosgrove]

>>> Eitan Adler 14-Feb-18 08:09 >>>
>
> Hi all,
>
> you may want the following patch. Previous discussion:
> https://lists.freebsd.org/pipermail/freebsd-numerics/2018-February/thread.html
>
> Original submission: https://github.com/freebsd/freebsd/pull/130
>
>
> ===
>
> As a component of atan2(y, x), the case of x == 1.0 is farmed out to
> atan(y). The current implementation of this comparison is vulnerable
> to signed integer underflow (that is, undefined behavior), and it's
> performed in a somewhat more complicated way than it need be. Change
> it to not be quite so cute, rather directly comparing the high/low
> bits of x to the specific IEEE-754 bit pattern that encodes 1.0.
>
> Note that while there are three different e_atan* files in the
> relevant directory, only this one needs fixing. e_atan2f.c already
> compares against the full bit pattern encoding 1.0f, while
> e_atan2l.cuses bitwise-ands/ors/nots and so doesn't require a change.
> ===
>
>
>
> Index: e_atan2.c
> ===
> RCS file: /cvs/src/lib/libm/src/e_atan2.c,v
> retrieving revision 1.13
> diff -u -r1.13 e_atan2.c
> --- e_atan2.c 12 Sep 2016 19:47:02 - 1.13
> +++ e_atan2.c 14 Feb 2018 08:06:05 -
> @@ -64,7 +64,7 @@
>   if(((ix|((lx|-lx)>>31))>0x7ff0)||
>  ((iy|((ly|-ly)>>31))>0x7ff0)) /* x or y is NaN */
>  return x+y;
> - if(((hx-0x3ff0)|lx)==0) return atan(y);   /* x=1.0 */
> + if(hx==0x3ff0&==0) return atan(y);   /* x=1.0 */

Wouldn't it work just as well, with fewer jumps, and match the original
intent, to replace the subtraction with xor?

 if(((hx^0x3ff0)|lx)==0) return atan(y);   /* x=1.0 */

>   m = ((hy>>31)&1)|((hx>>30)&2); /* 2*sign(x)+sign(y) */
>
>  /* when y = 0 */
>
>

Tom

Re: Basic SHA3 support

[Tom Cosgrove]

>>> "Todd C. Miller" 10-Jan-18 16:28 >>>
>
> On Wed, 10 Jan 2018 15:59:30 +0100, Jeremie Courreges-Anglas wrote:
>
> > Do we really want these?  For SHA-2, sha256 and sha512 are enough IMHO.
>
> Does anyone actually use the sha512 command?  I'd be fine with
> retiring the sha512 link and adding a sha3-256 one.
>
>  - todd

I'd like to keep the sha512 link - I do use it.

Tom

Re: [patch] Initialize "cur" to avoid undefined behavior is dmesg.c

[Tom Cosgrove]

> - free(allocated);
> + if (allocated)
> + free(allocated);

This is unnecessary, since free(NULL) is clearly defined as a no-op.
See the malloc(3) man page.

Tom

>>> Nan Xiao 4-Sep-17 12:11 >>>
>
> Hi tech@,
>
> This patch fixes the extreme case in dmesg.c: if memf or nlistf is not
> NULL, and "NOKVM" macro is defined.
>
> Current code in dmesg.c:
>
>   struct msgbuf cur;
>   
> Since "cur" is not initialized, so the following code has undefined
> behavior:
>
>   if (cur.msg_bufx >= cur.msg_bufs)
>   cur.msg_bufx = 0;
>   /*
>* The message buffer is circular; start at the read pointer, and
>* go to the write pointer - 1.
>*/
>   for (newl = skip = i = 0, p = bufdata + cur.msg_bufx;
>   i < cur.msg_bufs; i++, p++) {
>   .
>   }
>
> My patch can skip the whole loop, and the "dmesg" program just prints
> a newline:
>
>   if (!newl)
>   putchar('\n');
>
> Best Regards
> Nan Xiao
>
> Index: dmesg.c
> ===
> RCS file: /cvs/src/sbin/dmesg/dmesg.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 dmesg.c
> --- dmesg.c   1 Sep 2017 07:31:45 -   1.29
> +++ dmesg.c   4 Sep 2017 08:55:50 -
> @@ -65,12 +65,12 @@ main(int argc, char *argv[])
>   int ch, newl, skip, i;
>   char *p;
>   struct msgbuf cur;
> - char *memf, *nlistf, *bufdata = NULL;
> + char *memf = NULL, *nlistf = NULL, *bufdata = NULL;
>   char *allocated = NULL;
>   int startupmsgs = 0;
>   char buf[5];
>
> - memf = nlistf = NULL;
> + memset(, 0, sizeof(cur));
>   while ((ch = getopt(argc, argv, "sM:N:")) != -1)
>   switch(ch) {
>   case 's':
> @@ -184,7 +184,8 @@ main(int argc, char *argv[])
>   }
>   if (!newl)
>   putchar('\n');
> - free(allocated);
> + if (allocated)
> + free(allocated);
>   return (0);
>  }

Re: enum unsigned or not?

[Tom Cosgrove]

>>> patrick keshishian 31-Aug-17 20:40 >>>
>
> On Thu, Aug 31, 2017 at 12:13:19PM -0700, William Ahern wrote:
> > On Thu, Aug 31, 2017 at 02:08:07PM +0200, Otto Moerbeek wrote:
> > > Hi,
> > > 
> > > /usr/src/usr.sbin/sasyncd/carp.c:157:12: warning: comparison of
> > > unsigned enum expression < 0 is always false [-Wtautological-compare]
> > > if (state < 0 || state > FAIL)
> > > ~ ^ ~
> > > /usr/src/usr.sbin/sasyncd/carp.c:166:20: warning: comparison of
> > > unsigned enum expression < 0 is always false [-Wtautological-compare]
> > > if (current_state < 0 || current_state > FAIL) {
> > > ~ ^ ~
>
>   if (!(state >= INIT && state <= FAIL))
>   state = FAIL;
> and
>   if (!(current_sate >= INIT && current_state <= FAIL) {
>   log_err ...
>   ...
>   return;
>   }
>
> More better?

If the compiler is using an unsigned underlying type for the enum, the
comparison

state >= INIT

will give something along the lines of "expression >= 0 is always true"

I don't know what the best solution is here; I would defer to Otto!

Tom

> --patrick
>
> > > 
> > > this warning is a tiny bit interesting. A compiler is free to choose
> > > the type of the enum, as long as it can represent all given values.
> > > So another compiler might choose not to make it unsigned. So I came up
> > > with this fix that is not depending on the signedness of the type. But
> > > most of the time avoiding enum is better, I suppose.
> > 
> > It's free to choose the integer type of the enum, but enumeration members
> > (i.e. the constant identifiers) have int type.
> > 
> >   The identifiers in an enumerator list are declared as constants that have
> >   type int and may appear wherever such are permitted.
> > 
> >   C11 (N1570) 6.7.2.2p3.
> > 
> > Furthermore, the defining expression of the constant must be representable
> > as an int. 6.7.2.2p2.
> > 
> > I've always vascillated about which operand to cast, and to which type, when
> > silencing compilers' annoying warnings. But now that I've read the section
> > more closely I think I'll just cast the operand with enum type to int, all
> > things being equal.
> > 

Re: RFC 7217: random but stable addresses (take 2)

[Tom Cosgrove]

>>> Christian Weisgerber 14-Jul-17 23:04 >>>
>
> > secondly, im always wary of truncating hash output in case it throws
> > away some of the guarantees it's supposed to provide. if you cut
> > sha512 output down to an 8th of its size, is it 8 times easier to
> > calculate a collision, or more than 8 times easier? sha384 being a
> > truncation of sha512 kind of argues against this though.
>
> NIST FIPS 180-4 (the SHA-2 standard) says:
>
>   Some application may require a hash function with a message digest
>   length different than those provided by the hash functions in this
>   Standard. In such cases, a truncated message digest may be used,
>   whereby a hash function with a larger message digest length is
>   applied to the data to be hashed, and the resulting message digest
>   is truncated by selecting an appropriate number of the leftmost
>   bits. [...]
>
> (For some reason though the same standard specifies "SHA-512/t"
> hash functions, which are SHA-512 truncated to t bits, to use
> different initial hash values.  Maybe some mathematical rigor thing
> to distinguish truncation by the user from truncation inside the
> function?)

It is fine to truncate the output of a (good) hash function - see
this answer from Thomas Pornin on crypto.SE: 
https://crypto.stackexchange.com/a/163

However, when defining a new hash function as the truncation of the
output of an existing one (e.g. when using SHA256 to create a drop-in
replacement for a system that used SHA1 or MD5), it is considered
important to use a different set of IV constants - see this set of
slides from NIST: 
http://csrc.nist.gov/groups/ST/hash/documents/Kelsey_Truncation.pdf

No doubt I'll be shot down for the rest, but anway:

Obviously when truncating output down to 48 bits, the birthday attack
is only of the order of 2^24 bits, so finding colliions is not difficult.
But anyway, our output is public, and someone wishing to collide with us
doesn't have to use our mechanism, so collision resistance (in the usual
sense) is immaterial.

And nor (for the same reasons) are we worried about second preimage resistance.

So really, we're just using the hash function here as a PRF, to generate
random-looking but deterministic output from a given set of inputs.

In which case it doesn't really matter whether we use SHA512 or SipHash.

SipHash has some nice properties, but they're generally around the performance
side of things.  SHA512 is probably the more conservative choice, and
absolutely fine here.

Tom

Re: regarding OpenSSL License change

[Tom Cosgrove]

>>> Jimmy Hess 27-Mar-17 02:49 >>>
:
> silence does not generally grant permission. 

Since never grants permission.

> But the people in that project might be able to convincingly deliver some
> kind of argument that they've had implicit or "understood" permissions
> made at time of submission to use contributions however the project
> collectively agrees to use them.

Absolutely not.

When I contribute to an open source project, I do so under the terms of the
licences in the files I work on _at that time_.  If I completely rewrite or
add new files, I put those files under the standard licence used by the project,
and that code is then licenced in that (possibly different) way.

And the specific licence is important to me.  It is a significant factor in the
choice of which project to work on (which is why I choose to hack on OpenBSD
rather than, say, Linux).

The terms under which I contribute are those licences - there is no other
implied permission.  If anyone wants to change the licence used by code I have
contributed, they need my approval.  And if they want me to be accommodating,
there had better be a public discussion about alternative licences first.

Tom

Re: arm cpu.h rev 1.44 broken

[Tom Cosgrove]

Fixed, thanks

>>> Markus Hennecke 3-Mar-17 14:29 >>>
>
> There is a comma missing in rev 1.44:
>
> Index: cpu.h
> ===
> RCS file: /cvs/src/sys/arch/arm/include/cpu.h,v
> retrieving revision 1.44
> diff -u -p -r1.44 cpu.h
> --- cpu.h 2 Mar 2017 10:38:10 -   1.44
> +++ cpu.h 3 Mar 2017 14:26:27 -
> @@ -82,7 +82,7 @@
>   { 0, 0 }, \
>   { 0, 0 }, \
>   { "maxspeed", CTLTYPE_INT }, \
> - { "lidsuspend", CTLTYPE_INT } \
> + { "lidsuspend", CTLTYPE_INT }, \
>   { "lidaction", CTLTYPE_INT } \
>  }
>  

Re: Implement fork1_to_pid(). It's fork1(), but with pid as argument

[Tom Cosgrove]

>>> Philip Guenther 27-Feb-17 06:09 >>>
:
> I've now gone back and forth on this diff a few times.  I like the 
> simplfication of allocpid(), but then it seems like deck-chair shuffling 
> as the test is just moved to another function.
>
> But eliminating the magic-at-a-distance randompid frobbing is nice, as is 
> making it clear that there's really only one PID that will ever be 
> special.  Indeed, it would even allow kthreads to be created before init 
> (though what _else_ they depend on is unclear...).  Maybe setting the USB 
> tasks loose earlier would be useful, for example.  Hmm.
>
> Since I'm so mixed, do any other devs have an opinion?

I like the removal of the global randompid, and using a flag to ask for
PID 1 is much better than the original proposal of specifying the PID.

Since the bikeshed could always do with a new coat of paint, how about
renaming allocpid() to randompid()? :)

But you have an ok on the FORK_PID1 implementation and the man page change
you suggest.

Tom

> > > --- a/share/man/man9/fork1.9
> > > +++ b/share/man/man9/fork1.9
> > > @@ -109,6 +109,9 @@ must also be set.
> > >  .It Dv FORK_PTRACE
> > >  The child will start with tracing enabled, as if
> > >  ptrace(PT_TRACE_ME, 0, 0, 0) had been invoked in the child.
> > > +.It Dv FORK_PID1
> > > +Special flag to assign PID 1 for
> > > +.Xr init 8 process.
>
> I think this would be a nice place to explicitly mention the default is a 
> random pid, perhaps:
>   .It Dv FORK_PID1
>   The child is
>   .Xr init 8
>and is assigned PID 1 instead of a random PID.
>
> Philip

Re: ocspcheck typos

[Tom Cosgrove]

>>> Jason McIntyre 26-Jan-17 23:00 >>>
>
> On Thu, Jan 26, 2017 at 11:15:05PM +0100, Holger Mikolon wrote:
> > Hi,
> > 
> > below are two minor typo fixes: s/OSCP/OCSP/
> > 
> > Holger
> > ;-se
>
> fixed, thanks, plus one more in nc.
> jmc

There are also these in comments and warning messages.

ok?  (Asking as these aren't any of my usual areas)


Index: lib/libtls/tls_ocsp.c
===
RCS file: /cvs/src/lib/libtls/tls_ocsp.c,v
retrieving revision 1.9
diff -u -p -u -r1.9 tls_ocsp.c
--- lib/libtls/tls_ocsp.c   5 Nov 2016 15:13:26 -   1.9
+++ lib/libtls/tls_ocsp.c   27 Jan 2017 03:58:11 -
@@ -356,7 +356,7 @@ tls_ocsp_stapling_cb(SSL *ssl, void *arg
  * Public API
  */
 
-/* Retrieve OSCP URL from peer certificate, if present */
+/* Retrieve OCSP URL from peer certificate, if present */
 const char *
 tls_peer_ocsp_url(struct tls *ctx)
 {
Index: usr.sbin/httpd/parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.87
diff -u -p -u -r1.87 parse.y
--- usr.sbin/httpd/parse.y  5 Jan 2017 13:53:09 -   1.87
+++ usr.sbin/httpd/parse.y  27 Jan 2017 03:58:11 -
@@ -344,7 +344,7 @@ server  : SERVER optmatch STRING{
 
if (server_tls_load_ocsp(srv) == -1) {
yyerror("server \"%s\": failed to load "
-   "oscp staple", srv->srv_conf.name);
+   "ocsp staple", srv->srv_conf.name);
serverconfig_free(srv_conf);
free(srv);
YYERROR;
@@ -2069,7 +2069,7 @@ server_inherit(struct server *src, struc
}
 
if (server_tls_load_ocsp(dst) == -1) {
-   yyerror("failed to load oscp staple "
+   yyerror("failed to load ocsp staple "
"for server %s", dst->srv_conf.name);
serverconfig_free(>srv_conf);
free(dst);
Index: regress/usr.bin/openssl/appstest.sh
===
RCS file: /cvs/src/regress/usr.bin/openssl/appstest.sh,v
retrieving revision 1.1
diff -u -p -u -r1.1 appstest.sh
--- regress/usr.bin/openssl/appstest.sh 6 Nov 2016 11:56:43 -   1.1
+++ regress/usr.bin/openssl/appstest.sh 27 Jan 2017 03:58:11 -
@@ -847,7 +847,7 @@ ocsp_svr_pid=$!
 echo "ocsp server pid = [ $ocsp_svr_pid ]"
 sleep 1
 
-# send query to oscp server
+# send query to ocsp server
 start_message "ocsp ... send OCSP request to server"
 
 ocsp_qry=$user1_dir/ocsp_qry.der

Re: libcrypto: get rid of I386_ONLY

[Tom Cosgrove]

>>> Miod Vallat 4-Nov-16 08:53 >>>
>
> I386_ONLY was used to prefer a different assembler sequence in the
> sha512 code, which would be faster on 80386 processors, but slower on
> 80486 and above.
>
> This code path has never been enabled, and there are actually no plans
> to make libcrypto friendlier to genuine 80386 chips, so why bother
> keeping this - it's only contributing to obfuscation.

... not to mention that OpenBSD hasn't supported the 386 for a long time now

ok tom@

> Index: cryptlib.c
> ===
> RCS file: /cvs/src/lib/libcrypto/cryptlib.c,v
> retrieving revision 1.37
> diff -u -p -r1.37 cryptlib.c
> --- cryptlib.c13 Sep 2015 16:56:11 -  1.37
> +++ cryptlib.c4 Nov 2016 08:49:22 -
> @@ -635,7 +635,7 @@ OPENSSL_cpu_caps(void)
>   return *(uint64_t *)OPENSSL_ia32cap_P;
>  }
>  
> -#if defined(OPENSSL_CPUID_OBJ) && !defined(OPENSSL_NO_ASM) && 
> !defined(I386_ONLY)
> +#if defined(OPENSSL_CPUID_OBJ) && !defined(OPENSSL_NO_ASM)
>  #define OPENSSL_CPUID_SETUP
>  typedef unsigned long long IA32CAP;
>  void
> Index: md32_common.h
> ===
> RCS file: /cvs/src/lib/libcrypto/md32_common.h,v
> retrieving revision 1.21
> diff -u -p -r1.21 md32_common.h
> --- md32_common.h 4 Sep 2016 14:31:29 -   1.21
> +++ md32_common.h 4 Nov 2016 08:49:22 -
> @@ -152,8 +152,8 @@ static inline uint32_t ROTATE(uint32_t a
>  #if defined(DATA_ORDER_IS_BIG_ENDIAN)
>  
>  #if defined(__GNUC__) && __GNUC__>=2 && !defined(OPENSSL_NO_ASM) && 
> !defined(OPENSSL_NO_INLINE_ASM)
> -# if ((defined(__i386) || defined(__i386__)) && !defined(I386_ONLY)) || \
> -  (defined(__x86_64) || defined(__x86_64__))
> +# if (defined(__i386) || defined(__i386__) || \
> +  defined(__x86_64) || defined(__x86_64__))
>  /*
>   * This gives ~30-40% performance improvement in SHA-256 compiled
>   * with gcc [on P4]. Well, first macro to be frank. We can pull
> Index: arch/alpha/opensslconf.h
> ===
> RCS file: /cvs/src/lib/libcrypto/arch/alpha/opensslconf.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 opensslconf.h
> --- arch/alpha/opensslconf.h  19 Jun 2015 06:05:11 -  1.7
> +++ arch/alpha/opensslconf.h  4 Nov 2016 08:49:22 -
> @@ -1,9 +1,6 @@
>  #include 
>  /* crypto/opensslconf.h.in */
>  
> -/* Generate 80386 code? */
> -#undef I386_ONLY
> -
>  #if defined(HEADER_CRYPTLIB_H) && !defined(OPENSSLDIR)
>  #define OPENSSLDIR "/etc/ssl"
>  #endif
> Index: arch/amd64/opensslconf.h
> ===
> RCS file: /cvs/src/lib/libcrypto/arch/amd64/opensslconf.h,v
> retrieving revision 1.9
> diff -u -p -r1.9 opensslconf.h
> --- arch/amd64/opensslconf.h  19 Jun 2015 06:05:11 -  1.9
> +++ arch/amd64/opensslconf.h  4 Nov 2016 08:49:22 -
> @@ -1,9 +1,6 @@
>  #include 
>  /* crypto/opensslconf.h.in */
>  
> -/* Generate 80386 code? */
> -#undef I386_ONLY
> -
>  #if defined(HEADER_CRYPTLIB_H) && !defined(OPENSSLDIR)
>  #define OPENSSLDIR "/etc/ssl"
>  #endif
> Index: arch/arm/opensslconf.h
> ===
> RCS file: /cvs/src/lib/libcrypto/arch/arm/opensslconf.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 opensslconf.h
> --- arch/arm/opensslconf.h19 Jun 2015 06:05:11 -  1.7
> +++ arch/arm/opensslconf.h4 Nov 2016 08:49:22 -
> @@ -1,9 +1,6 @@
>  #include 
>  /* crypto/opensslconf.h.in */
>  
> -/* Generate 80386 code? */
> -#undef I386_ONLY
> -
>  #if defined(HEADER_CRYPTLIB_H) && !defined(OPENSSLDIR)
>  #define OPENSSLDIR "/etc/ssl"
>  #endif
> Index: arch/hppa/opensslconf.h
> ===
> RCS file: /cvs/src/lib/libcrypto/arch/hppa/opensslconf.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 opensslconf.h
> --- arch/hppa/opensslconf.h   19 Jun 2015 06:05:11 -  1.7
> +++ arch/hppa/opensslconf.h   4 Nov 2016 08:49:22 -
> @@ -1,9 +1,6 @@
>  #include 
>  /* crypto/opensslconf.h.in */
>  
> -/* Generate 80386 code? */
> -#undef I386_ONLY
> -
>  #if defined(HEADER_CRYPTLIB_H) && !defined(OPENSSLDIR)
>  #define OPENSSLDIR "/etc/ssl"
>  #endif
> Index: arch/i386/opensslconf.h
> ===
> RCS file: /cvs/src/lib/libcrypto/arch/i386/opensslconf.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 opensslconf.h
> --- arch/i386/opensslconf.h   19 Jun 2015 06:05:11 -  1.7
> +++ arch/i386/opensslconf.h   4 Nov 2016 08:49:22 -
> @@ -1,9 +1,6 @@
>  #include 
>  /* crypto/opensslconf.h.in */
>  
> -/* Generate 80386 code? */
> -#undef I386_ONLY
> -
>  #if defined(HEADER_CRYPTLIB_H) && !defined(OPENSSLDIR)
>  #define OPENSSLDIR "/etc/ssl"
>  #endif
> Index: arch/m88k/opensslconf.h
> 

Re: mount(8): strlen + malloc + snprintf == asprintf

[Tom Cosgrove]

>>> Ali H. Fardan <r...@firemail.cc> 5-Sep-16 09:09 >>>
>
> On 2016-09-05 11:03, Tom Cosgrove wrote:
> :
> > It does allocate the correct buffer size.  It's got all the
> > information it needs to do that with the format string and the
> > parameters.  Then it returns the buffer address via the `ret'
> > argument.
> >
> > If you don't believe us, read the source code and tell us where we
> > are wrong.
> > 
> > Tom
>
> then that patch does weaken security, the buffer can overflow.

asprintf() allocates the buffer, of the size it needs.  It can't overflow.
It makes no change to security.

The patch is fine - you'll notice it's already been committed.

Tom

Re: mount(8): strlen + malloc + snprintf == asprintf

[Tom Cosgrove]

>>> Ali H. Fardan  5-Sep-16 08:47 >>>
>
> On 2016-09-05 10:44, David Gwynne wrote:
> >> On 5 Sep 2016, at 17:39, Ali H. Fardan  wrote:
> >> 
> >> and why is he telling me this? I just said if the destination is a
> >> pointer to char, how would a function automagically allocate a size
> >> for it?
> > 
> > its not a pointer to a char, its a pointer to a char pointer:
> > 
> > as per the man page:
> > 
> >  int
> >  asprintf(char **ret, const char *format, ...);
> > 
> > dlg
>
> Still doesn't mean that it can automagically allocate a correct
> buffer size.

It does allocate the correct buffer size.  It's got all the information it
needs to do that with the format string and the parameters.  Then it returns
the buffer address via the `ret' argument.

If you don't believe us, read the source code and tell us where we are wrong.

Tom

Re: better return type for usqrt() in factor(6)

[Tom Cosgrove]

ok tom@

>>> Theo Buehler 1-Sep-16 06:36 >>>
>
> In factor(6), there is the line
>
> 216   stop = usqrt(val) + 1;
>
> where the u_int64_t stop is the upper bound for the sieve of
> Eratosthenes (I cautiously added 1 to be sure to be on the safe side).
> Unfortunately, the right hand side may now overflow for large enough
> val because my brilliant former self chose usqrt() to be of type
> u_int32_t usqrt(u_sqrt64_t); ...
>
> Example from http://cvsweb.netbsd.org/bsdweb.cgi/src/games/primes/pattern.c
>
> $ printf "%u\n" $((139646831 * 132095686967))
> 18446744073709551577
> $ factor 18446744073709551577
> 18446744073709551577
>
> With the patch below:
>
> $ obj/factor
> 18446744073709551577: 139646831 132095686967
>
> as it should be.
>
> Index: factor.c
> ===
> RCS file: /var/cvs/src/games/factor/factor.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 factor.c
> --- factor.c  14 Aug 2016 18:34:48 -  1.29
> +++ factor.c  1 Sep 2016 04:56:00 -
> @@ -75,7 +75,7 @@ extern const int pattern_size;
>  
>  static void  pr_fact(u_int64_t); /* print factors of a value */
>  static void  pr_bigfact(u_int64_t);
> -static u_int32_t usqrt(u_int64_t);
> +static u_int64_t usqrt(u_int64_t);
>  static void __dead   usage(void);
>  
>  int
> @@ -284,7 +284,7 @@ pr_bigfact(u_int64_t val) /* Factor this
>  }
>  
>  /* Code taken from ping.c */
> -static u_int32_t
> +static u_int64_t
>  usqrt(u_int64_t n)
>  {
>   u_int64_t y, x = 1;
> @@ -299,7 +299,7 @@ usqrt(u_int64_t n)
>   x /= 2;
>   } while (((y < x) && (x - y) > 1) || (y - x) > 1);
>  
> - return (u_int32_t)x;
> + return x;
>  }
>  
>  static void __dead

Remove unused function ether_cmp() from smc91cxx.c

[Tom Cosgrove]

Index: sys/dev/ic/smc91cxx.c
===
RCS file: /home/OpenBSD/cvs/src/sys/dev/ic/smc91cxx.c,v
retrieving revision 1.47
diff -u -p -r1.47 smc91cxx.c
--- sys/dev/ic/smc91cxx.c   13 Apr 2016 10:49:26 -  1.47
+++ sys/dev/ic/smc91cxx.c   12 Jul 2016 14:32:13 -
@@ -181,18 +181,6 @@ void   smc91cxx_resume(struct smc91cxx_sof
 void   smc91cxx_watchdog(struct ifnet *);
 intsmc91cxx_ioctl(struct ifnet *, u_long, caddr_t);
 
-static __inline int ether_cmp(void *, void *);
-static __inline int
-ether_cmp(va, vb)
-   void *va, *vb;
-{
-   u_int8_t *a = va;
-   u_int8_t *b = vb;
-
-   return ((a[5] != b[5]) || (a[4] != b[4]) || (a[3] != b[3]) ||
-   (a[2] != b[2]) || (a[1] != b[1]) || (a[0] != b[0]));
-}
-
 void
 smc91cxx_attach(sc, myea)
struct smc91cxx_softc *sc;

Unsigned variables can't be < 0

[Tom Cosgrove]

... and size_t is unsigned.


Index: sys/dev/wscons/wsmouse.c
===
RCS file: /home/OpenBSD/cvs/src/sys/dev/wscons/wsmouse.c,v
retrieving revision 1.31
diff -u -p -U8 -r1.31 wsmouse.c
--- sys/dev/wscons/wsmouse.c5 Jul 2016 19:33:14 -   1.31
+++ sys/dev/wscons/wsmouse.c12 Jul 2016 15:12:15 -
@@ -1319,17 +1319,17 @@ wsmouse_init_scaling(struct wsmouseinput
 void
 wsmouse_set_param(struct device *sc, size_t param, int value)
 {
struct wsmouseinput *input =
&((struct wsmouse_softc *) sc)->input;
struct wsmouseparams *params = >params;
int *p;
 
-   if (param < 0 || param > WSMPARAM_LASTFIELD) {
+   if (param > WSMPARAM_LASTFIELD) {
printf("wsmouse_set_param: invalid parameter type\n");
return;
}
 
p = (int *) (((void *) params) + param);
*p = value;
 
if (IS_WSMFLTR_PARAM(param)) {

Re: Fix return value of OF_getprop() when name has a '@' in it

[Tom Cosgrove]

>>> Tom Cosgrove 12-Jun-16 22:05 >>>
>
> In OF_getprop(), if the "name" property doesn't exist it is synthesised
> from the unit name.  If that synthesised property has an "@" in it, we
> truncate the name just before the "@", but we currently continue to return
> the full length of the unit name.
>
> This diff returns the length of the truncated name.
>
> Thanks
>
> Tom

Updated diff following the recent commit to fdt.c

Ensures that OF_getprop() returns the same length value that OF_getproplen() did
when it's called on a node name with '@' in it

(Also implemented slightly differently this time, keeping a single 'return'
and instead re-calculating 'len'.)

Thanks

Tom


Index: sys/dev/ofw/fdt.c
===
RCS file: /home/OpenBSD/cvs/src/sys/dev/ofw/fdt.c,v
retrieving revision 1.13
diff -u -p -r1.13 fdt.c
--- sys/dev/ofw/fdt.c   14 Jun 2016 10:03:51 -  1.13
+++ sys/dev/ofw/fdt.c   14 Jun 2016 13:29:39 -
@@ -782,8 +782,10 @@ OF_getprop(int handle, char *prop, void 
if (data) {
len = strlcpy(buf, data, buflen);
data = strchr(buf, '@');
-   if (data)
+   if (data) {
*data = 0;
+   len = data - (char *)buf;
+   }
return len + 1;
}
}

Fix return value of OF_getprop() when name has a '@' in it

[Tom Cosgrove]

In OF_getprop(), if the "name" property doesn't exist it is synthesised
from the unit name.  If that synthesised property has an "@" in it, we
truncate the name just before the "@", but we currently continue to return
the full length of the unit name.

This diff returns the length of the truncated name.

Thanks

Tom


Index: sys/dev/ofw/fdt.c
===
RCS file: /home/OpenBSD/cvs/src/sys/dev/ofw/fdt.c,v
retrieving revision 1.12
diff -u -p -u -r1.12 fdt.c
--- sys/dev/ofw/fdt.c   12 Jun 2016 12:55:42 -  1.12
+++ sys/dev/ofw/fdt.c   12 Jun 2016 20:59:31 -
@@ -782,8 +782,10 @@ OF_getprop(int handle, char *prop, void 
if (data) {
len = strlcpy(buf, data, buflen);
data = strchr(buf, '@');
-   if (data)
+   if (data) {
*data = 0;
+   return data - (char *)buf + 1;
+   }
return len + 1;
}
}

Fix typos in comments in amd64/stand

[Tom Cosgrove]

Thanks

Tom


Index: sys/arch/amd64/stand/efiboot/efiboot.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
retrieving revision 1.12
diff -u -p -u -r1.12 efiboot.c
--- sys/arch/amd64/stand/efiboot/efiboot.c  15 May 2016 22:48:02 -  
1.12
+++ sys/arch/amd64/stand/efiboot/efiboot.c  10 Jun 2016 18:14:14 -
@@ -466,7 +466,7 @@ efi_cons_getshifts(dev_t dev)
return (0);
 }
 
-/* XXX: serial console is not supporte yet */
+/* XXX: serial console is not supported yet */
 int comspeed = 9600;
 int com_addr = -1;
 int com_speed = -1;
@@ -477,7 +477,7 @@ int com_speed = -1;
 /*
  * ACPI GUID is confusing in UEFI spec.
  * {EFI_,}_ACPI_20_TABLE_GUID or EFI_ACPI_TABLE_GUID means
- * ACPI 2.0 or abobe.
+ * ACPI 2.0 or above.
  */
 static EFI_GUID acpi_guid = ACPI_20_TABLE_GUID;
 static EFI_GUID smbios_guid = SMBIOS_TABLE_GUID;
Index: sys/arch/amd64/stand/libsa/cmd_i386.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/cmd_i386.c,v
retrieving revision 1.9
diff -u -p -u -r1.9 cmd_i386.c
--- sys/arch/amd64/stand/libsa/cmd_i386.c   8 Nov 2015 00:42:39 -   
1.9
+++ sys/arch/amd64/stand/libsa/cmd_i386.c   10 Jun 2016 18:14:14 -
@@ -183,7 +183,7 @@ Xmemory(void)
p++;
}
 
-   /* Handle (possibly non-existant) address part */
+   /* Handle (possibly non-existent) address part */
switch (*p) {
case '@':
addr = strtoll(p + 1, NULL, 0);
Index: sys/arch/amd64/stand/libsa/memprobe.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/memprobe.c,v
retrieving revision 1.16
diff -u -p -u -r1.16 memprobe.c
--- sys/arch/amd64/stand/libsa/memprobe.c   8 Oct 2015 14:46:05 -   
1.16
+++ sys/arch/amd64/stand/libsa/memprobe.c   10 Jun 2016 18:14:14 -
@@ -281,7 +281,7 @@ memprobe(void)
 * Compute compatibility values:
 * cnvmem -- is the upper boundary of conventional
 *  memory (below IOM_BEGIN (=640k))
-* extmem -- is the size of the contignous extended
+* extmem -- is the size of the contiguous extended
 *  memory segment starting at 1M
 *
 * We ignore "good" memory in the 640K-1M hole.
@@ -346,7 +346,7 @@ mem_limit(long long ml)
if (p->type != BIOS_MAP_FREE)
continue;
 
-   /* Wholy above limit, nuke it */
+   /* Wholly above limit, nuke it */
if ((sp >= ml) && (ep >= ml)) {
bcopy (p + 1, p, (char *)bios_memmap +
   sizeof(bios_memmap) - (char *)p);
Index: sys/arch/amd64/stand/libsa/pxe.h
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/pxe.h,v
retrieving revision 1.6
diff -u -p -u -r1.6 pxe.h
--- sys/arch/amd64/stand/libsa/pxe.h2 Mar 2011 07:15:45 -   1.6
+++ sys/arch/amd64/stand/libsa/pxe.h10 Jun 2016 18:14:14 -
@@ -346,7 +346,7 @@ typedef struct {
 #  define PXENV_UNDI_ISR_OUT_NOT_OUTS  1
 
/*
-* one of these will bre returnd for PXEND_UNDI_ISR_IN_PROCESS
+* one of these will be returned for PXEND_UNDI_ISR_IN_PROCESS
 * and PXENV_UNDI_ISR_IN_GET_NEXT
 */
 #  define PXENV_UNDI_ISR_OUT_DONE  0

Fix typos in comments in i386/stand

[Tom Cosgrove]

Thanks

Tom


Index: sys/arch/i386/stand/libsa/apmprobe.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/i386/stand/libsa/apmprobe.c,v
retrieving revision 1.18
diff -u -p -u -r1.18 apmprobe.c
--- sys/arch/i386/stand/libsa/apmprobe.c8 Jul 2014 13:31:30 -   
1.18
+++ sys/arch/i386/stand/libsa/apmprobe.c10 Jun 2016 18:06:23 -
@@ -27,7 +27,7 @@
  */
 /*
  * APM derived from: apm_init.S, LP (Laptop Package)
- * wich contained this:
+ * which contained this:
  * Copyright (C) 1994 by HOSOKAWA, Tatsumi 
  *
  */
Index: sys/arch/i386/stand/libsa/cmd_i386.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/i386/stand/libsa/cmd_i386.c,v
retrieving revision 1.35
diff -u -p -u -r1.35 cmd_i386.c
--- sys/arch/i386/stand/libsa/cmd_i386.c18 Sep 2015 13:30:56 -  
1.35
+++ sys/arch/i386/stand/libsa/cmd_i386.c10 Jun 2016 18:06:23 -
@@ -179,7 +179,7 @@ Xmemory(void)
p++;
}
 
-   /* Handle (possibly non-existant) address part */
+   /* Handle (possibly non-existent) address part */
switch (*p) {
case '@':
addr = strtoll(p + 1, NULL, 0);
Index: sys/arch/i386/stand/libsa/memprobe.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/i386/stand/libsa/memprobe.c,v
retrieving revision 1.56
diff -u -p -u -r1.56 memprobe.c
--- sys/arch/i386/stand/libsa/memprobe.c8 Oct 2015 14:46:05 -   
1.56
+++ sys/arch/i386/stand/libsa/memprobe.c10 Jun 2016 18:06:23 -
@@ -286,7 +286,7 @@ memprobe(void)
 * Compute compatibility values:
 * cnvmem -- is the upper boundary of conventional
 *  memory (below IOM_BEGIN (=640k))
-* extmem -- is the size of the contignous extended
+* extmem -- is the size of the contiguous extended
 *  memory segment starting at 1M
 *
 * We ignore "good" memory in the 640K-1M hole.
@@ -338,7 +338,7 @@ mem_limit(long long ml)
if (p->type != BIOS_MAP_FREE)
continue;
 
-   /* Wholy above limit, nuke it */
+   /* Wholly above limit, nuke it */
if ((sp >= ml) && (ep >= ml)) {
bcopy (p + 1, p, (char *)bios_memmap +
   sizeof(bios_memmap) - (char *)p);
Index: sys/arch/i386/stand/libsa/pxe.h
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/i386/stand/libsa/pxe.h,v
retrieving revision 1.6
diff -u -p -u -r1.6 pxe.h
--- sys/arch/i386/stand/libsa/pxe.h 2 Mar 2011 07:15:45 -   1.6
+++ sys/arch/i386/stand/libsa/pxe.h 10 Jun 2016 18:06:23 -
@@ -346,7 +346,7 @@ typedef struct {
 #  define PXENV_UNDI_ISR_OUT_NOT_OUTS  1
 
/*
-* one of these will bre returnd for PXEND_UNDI_ISR_IN_PROCESS
+* one of these will be returned for PXEND_UNDI_ISR_IN_PROCESS
 * and PXENV_UNDI_ISR_IN_GET_NEXT
 */
 #  define PXENV_UNDI_ISR_OUT_DONE  0

Typo in comment in arm/undefined.c

[Tom Cosgrove]

Thanks

Tom


Index: sys/arch/arm/arm/undefined.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/arm/arm/undefined.c,v
retrieving revision 1.7
diff -u -p -u -r1.7 undefined.c
--- sys/arch/arm/arm/undefined.c31 Jan 2016 00:14:50 -  1.7
+++ sys/arch/arm/arm/undefined.c10 Jun 2016 00:15:04 -
@@ -192,7 +192,7 @@ undefinedinstruction(trapframe_t *frame)
 
/*
 * According to the datasheets you only need to look at bit 27 of the
-* instruction to tell the difference between and undefined
+* instruction to tell the difference between an undefined
 * instruction and a coprocessor instruction following an undefined
 * instruction trap.
 */

Spelling correction for www/armv7.html

[Tom Cosgrove]

Thanks

Tom


Index: www/armv7.html
===
RCS file: /home/OpenBSD/cvs/www/armv7.html,v
retrieving revision 1.26
diff -u -p -u -r1.26 armv7.html
--- www/armv7.html  29 May 2016 14:44:33 -  1.26
+++ www/armv7.html  9 Jun 2016 08:14:00 -
@@ -62,8 +62,8 @@ bundles various platforms sharing the AR
 fact that there are many System on a Chips (SoC) around,
 OpenBSD/armv7 differentiates between various SoCs and may have a
 different level of support between them. All devices based on the
-i.MX6 are refered to as imx, all devices based on A1x/A20 are
-refered to as sunxi. The boards with an OMAP 3/4 SoC are
+i.MX6 are referred to as imx, all devices based on A1x/A20 are
+referred to as sunxi. The boards with an OMAP 3/4 SoC are
 subdivided into am335x (for BeagleBone), beagle (for
 BeagleBoard) and panda (for PandaBoard).
 

Update floppy58 and miniroot58 to *59.fs in www/

[Tom Cosgrove]

... to bring in line with sparc64.html

(Didn't update vax.html as it's been discontinued)

Thanks

Tom


Index: www/alpha.html
===
RCS file: /home/OpenBSD/cvs/www/alpha.html,v
retrieving revision 1.276
diff -u -p -u -r1.276 alpha.html
--- www/alpha.html  8 Apr 2016 01:58:04 -   1.276
+++ www/alpha.html  8 Jun 2016 21:06:37 -
@@ -242,7 +242,7 @@ There are several installation media pro
   look at the
   http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/alpha/conf/RAMDISKBIG?rev=HEAD;>RAMDISKBIG
   kernel configuration file.
-  Floppy A (floppy58.fs)
+  Floppy A (floppy59.fs)
   
   This 1.44MB floppy image supports the following alpha hardware:
   
@@ -262,7 +262,7 @@ There are several installation media pro
   look at the
   http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/alpha/conf/RAMDISK?rev=HEAD;>RAMDISK
   kernel configuration file.
-  Floppy B (floppyB58.fs)
+  Floppy B (floppyB59.fs)
   
   This 1.44MB floppy image supports the following alpha hardware:
   
Index: www/amd64.html
===
RCS file: /home/OpenBSD/cvs/www/amd64.html,v
retrieving revision 1.262
diff -u -p -u -r1.262 amd64.html
--- www/amd64.html  8 Apr 2016 01:58:04 -   1.262
+++ www/amd64.html  8 Jun 2016 21:07:00 -
@@ -109,11 +109,11 @@ There are several installation media pro
   For the latest list of drivers available on this image, take a look at the
   http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/amd64/conf/RAMDISK_CD?rev=HEAD;>RAMDISK_CD
   kernel configuration file.
-  Disk Image (miniroot58.fs)
+  Disk Image (miniroot59.fs)
   
   The same installer as the CD, but in a form suitable for creating bootable
   hard drives or USB flash drives.
-  Floppy A (floppy58.fs)
+  Floppy A (floppy59.fs)
   
   This 1.44MB floppy image contains the most common drivers.
   It is designed to cover the most typical PC. As a general rule, you will
Index: www/i386.html
===
RCS file: /home/OpenBSD/cvs/www/i386.html,v
retrieving revision 1.740
diff -u -p -u -r1.740 i386.html
--- www/i386.html   8 Apr 2016 01:58:04 -   1.740
+++ www/i386.html   8 Jun 2016 21:07:04 -
@@ -136,11 +136,11 @@ There are several installation media pro
   For the latest list of drivers available on this image, take a look at the
   http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/i386/conf/RAMDISK_CD?rev=HEAD;>RAMDISK_CD
   kernel configuration file.
-  Disk Image (miniroot58.fs)
+  Disk Image (miniroot59.fs)
   
   The same installer as the CD, but in a form suitable for creating bootable
   hard drives or USB flash drives.
-  Floppy (floppy58.fs)
+  Floppy (floppy59.fs)
   
   This 1.44MB floppy image contains the most common drivers.
   It is designed to cover the most typical PC. As a general rule, you will
Index: www/sparc.html
===
RCS file: /home/OpenBSD/cvs/www/sparc.html,v
retrieving revision 1.226
diff -u -p -u -r1.226 sparc.html
--- www/sparc.html  8 Apr 2016 01:58:04 -   1.226
+++ www/sparc.html  8 Jun 2016 21:07:19 -
@@ -535,7 +535,7 @@ kernel configuration file.
   boot cdrom 5.7/sparc/bsd.rd
 
   
-  Floppy (floppy58.fs)
+  Floppy (floppy59.fs)
   
   Booting off the floppy provides a small ffs filesystem with a kernel
   containing drivers for the most popular devices found on SPARC machines.
@@ -547,7 +547,7 @@ kernel configuration file.
   boot floppy
 
   
-  Miniroot (miniroot58.fs)
+  Miniroot (miniroot59.fs)
   
   The miniroot provides the same installation environment as the bootable CD,
   and is intended for easy bootstrap if there is already an operating system

No need to second guess SXIE_ROUNDUP

[Tom Cosgrove]

If (pktlen & 3) == 0, SXIE_ROUNDUP returns pktlen anyway (that's its job):
it's defined as

#define SXIE_ROUNDUP(size, unit) (((size) + (unit) - 1) & ~((unit) - 1))

Thanks

Tom


Index: sys/arch/armv7/sunxi/sxie.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/armv7/sunxi/sxie.c,v
retrieving revision 1.14
diff -u -p -u -r1.14 sxie.c
--- sys/arch/armv7/sunxi/sxie.c 13 Apr 2016 11:34:00 -  1.14
+++ sys/arch/armv7/sunxi/sxie.c 8 Jun 2016 23:03:46 -
@@ -606,10 +606,7 @@ trynext:
m_adj(m, ETHER_ALIGN);
 
/* read the actual packet from fifo XXX through 'align buffer'.. */
-   if (pktlen & 3)
-   rlen = SXIE_ROUNDUP(pktlen, 4);
-   else
-   rlen = pktlen;
+   rlen = SXIE_ROUNDUP(pktlen, 4);
bus_space_read_multi_4(sc->sc_iot, sc->sc_ioh,
SXIE_RXIO, (uint32_t *)[0], rlen >> 2);
memcpy(mtod(m, char *), (char *)[0], pktlen);

Uninitialised variable in sys/arch/armv7/exynos/crosec.c

[Tom Cosgrove]

Hi

I can't test this :) but it might bite someone who was trying to hack
in this area.

Thanks

Tom


Index: sys/arch/armv7/exynos/crosec.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/armv7/exynos/crosec.c,v
retrieving revision 1.1
diff -u -p -u -r1.1 crosec.c
--- sys/arch/armv7/exynos/crosec.c  26 Jan 2015 02:48:24 -  1.1
+++ sys/arch/armv7/exynos/crosec.c  8 Jun 2016 19:52:58 -
@@ -222,7 +222,7 @@ cros_ec_command_inptr(struct cros_ec_sof
int ret;
 
delay(5);
-   cros_ec_send_command(sc, EC_CMD_GET_COMMS_STATUS, 0,
+   ret = cros_ec_send_command(sc, EC_CMD_GET_COMMS_STATUS, 
0,
NULL, 0,
(uint8_t **), sizeof(*resp));
if (ret < 0)

Re: 'continue' to appease style gods in i386,amd64 libsa

[Tom Cosgrove]

Hi

> Two nits inline:

Thanks for the feedback.  Updated diff below.

Tom

>>> <pabl...@gmail.com> 8-Jun-16 11:52 >>>
>
> Hi Tom,
>
> Two nits inline:
>
> On Tue, Jun 7, 2016 at 9:47 PM, Tom Cosgrove 
> <tom.cosgr...@arches-consulting.com> wrote:
>
> >>>> Tom Cosgrove 6-Jun-16 21:07 >>>
> >>
> >> As per subject, a couple of empty loop bodies in the i396 and amd64 boot 
> >> blocks.
> >>
> >> Diff below.
> >>
> >> Tom
> >
> > Subsequently found a few more, and a handful of trailing whitespaces.
> >
> > Updated diff below.


Index: sys/arch/amd64/stand/efiboot/efidev.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/efiboot/efidev.c,v
retrieving revision 1.18
diff -u -p -u -r1.18 efidev.c
--- sys/arch/amd64/stand/efiboot/efidev.c   6 May 2016 03:13:52 -   
1.18
+++ sys/arch/amd64/stand/efiboot/efidev.c   8 Jun 2016 11:32:52 -
@@ -568,7 +568,8 @@ efiopen(struct open_file *f, ...)
}
 #endif
for (maj = 0; maj < nbdevs &&
-   strncmp(dev, bdevs[maj], devlen); maj++);
+   strncmp(dev, bdevs[maj], devlen); maj++)
+   continue;
if (maj >= nbdevs) {
printf("Unknown device: ");
for (cp = *file; *cp != ':'; cp++)
Index: sys/arch/amd64/stand/libsa/bioscons.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/bioscons.c,v
retrieving revision 1.11
diff -u -p -u -r1.11 bioscons.c
--- sys/arch/amd64/stand/libsa/bioscons.c   27 May 2016 05:37:51 -  
1.11
+++ sys/arch/amd64/stand/libsa/bioscons.c   8 Jun 2016 11:32:52 -
@@ -155,7 +155,7 @@ com_init(struct consdev *cn)
 
/* A few ms delay for the chip, using the getsecs() API */
while (!(i++ % 1000) && getsecs() < tt)
-   ;
+   continue;
 
/* drain the input buffer */
while (inb(port + com_lsr) & LSR_RXRDY)
@@ -171,7 +171,7 @@ com_getc(dev_t dev)
return (inb(port + com_lsr) & LSR_RXRDY);
 
while ((inb(port + com_lsr) & LSR_RXRDY) == 0)
-   ;
+   continue;
 
return (inb(port + com_data) & 0xff);
 }
@@ -237,7 +237,7 @@ com_putc(dev_t dev, int c)
int port = (com_addr == -1) ? comports[minor(dev)] : com_addr;
 
while ((inb(port + com_lsr) & LSR_TXRDY) == 0)
-   ;
+   continue;
 
outb(port + com_data, c);
 }
Index: sys/arch/amd64/stand/libsa/biosdev.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/biosdev.c,v
retrieving revision 1.27
diff -u -p -u -r1.27 biosdev.c
--- sys/arch/amd64/stand/libsa/biosdev.c1 Oct 2015 20:28:12 -   
1.27
+++ sys/arch/amd64/stand/libsa/biosdev.c8 Jun 2016 11:32:52 -
@@ -560,7 +560,8 @@ biosopen(struct open_file *f, ...)
 #endif
 
for (maj = 0; maj < nbdevs &&
-   strncmp(dev, bdevs[maj], devlen); maj++);
+   strncmp(dev, bdevs[maj], devlen); maj++)
+   continue;
if (maj >= nbdevs) {
printf("Unknown device: ");
for (cp = *file; *cp != ':'; cp++)
@@ -671,7 +672,8 @@ biosdisk_err(u_int error)
register const u_char *p = bidos_errs;
 
while (*p && *p != error)
-   while (*p++);
+   while (*p++)
+   continue;
 
return ++p;
 }
@@ -703,7 +705,8 @@ biosdisk_errno(u_int error)
if (error == 0)
return 0;
 
-   for (p = tab; p->error && p->error != error; p++);
+   for (p = tab; p->error && p->error != error; p++)
+   continue;
 
return p->errno;
 }
Index: sys/arch/amd64/stand/libsa/diskprobe.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/diskprobe.c,v
retrieving revision 1.16
diff -u -p -u -r1.16 diskprobe.c
--- sys/arch/amd64/stand/libsa/diskprobe.c  2 Sep 2015 01:52:26 -   
1.16
+++ sys/arch/amd64/stand/libsa/diskprobe.c  8 Jun 2016 11:32:52 -
@@ -266,7 +266,7 @@ diskprobe(void)
 
/* Checksumming of hard disks */
for (i = 0; disksum(i++) && i < MAX_CKSUMLEN; )
-   ;
+   continue;
bios_cksumlen = i;
 
/* Get space for passing bios_diskinfo stuff to kernel */
Index: sys/arch/amd64/stand/libsa/gateA20.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/gateA20.c,v
retrieving revision 1.2
diff -u -p -u -r1.2

Re: 'continue' to appease style gods in i386,amd64 libsa

[Tom Cosgrove]

>>> Tom Cosgrove 6-Jun-16 21:07 >>>
>
> As per subject, a couple of empty loop bodies in the i396 and amd64 boot 
> blocks.
>
> Diff below.
>
> Tom

Subsequently found a few more, and a handful of trailing whitespaces.

Updated diff below.

Thanks

Tom


Index: sys/arch/amd64/stand/efiboot/efidev.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/efiboot/efidev.c,v
retrieving revision 1.18
diff -u -p -u -r1.18 efidev.c
--- sys/arch/amd64/stand/efiboot/efidev.c   6 May 2016 03:13:52 -   
1.18
+++ sys/arch/amd64/stand/efiboot/efidev.c   7 Jun 2016 20:40:16 -
@@ -568,7 +568,7 @@ efiopen(struct open_file *f, ...)
}
 #endif
for (maj = 0; maj < nbdevs &&
-   strncmp(dev, bdevs[maj], devlen); maj++);
+   strncmp(dev, bdevs[maj], devlen); maj++) continue;
if (maj >= nbdevs) {
printf("Unknown device: ");
for (cp = *file; *cp != ':'; cp++)
Index: sys/arch/amd64/stand/libsa/bioscons.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/bioscons.c,v
retrieving revision 1.11
diff -u -p -u -r1.11 bioscons.c
--- sys/arch/amd64/stand/libsa/bioscons.c   27 May 2016 05:37:51 -  
1.11
+++ sys/arch/amd64/stand/libsa/bioscons.c   7 Jun 2016 20:40:16 -
@@ -155,7 +155,7 @@ com_init(struct consdev *cn)
 
/* A few ms delay for the chip, using the getsecs() API */
while (!(i++ % 1000) && getsecs() < tt)
-   ;
+   continue;
 
/* drain the input buffer */
while (inb(port + com_lsr) & LSR_RXRDY)
@@ -171,7 +171,7 @@ com_getc(dev_t dev)
return (inb(port + com_lsr) & LSR_RXRDY);
 
while ((inb(port + com_lsr) & LSR_RXRDY) == 0)
-   ;
+   continue;
 
return (inb(port + com_data) & 0xff);
 }
@@ -237,7 +237,7 @@ com_putc(dev_t dev, int c)
int port = (com_addr == -1) ? comports[minor(dev)] : com_addr;
 
while ((inb(port + com_lsr) & LSR_TXRDY) == 0)
-   ;
+   continue;
 
outb(port + com_data, c);
 }
Index: sys/arch/amd64/stand/libsa/biosdev.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/biosdev.c,v
retrieving revision 1.27
diff -u -p -u -r1.27 biosdev.c
--- sys/arch/amd64/stand/libsa/biosdev.c1 Oct 2015 20:28:12 -   
1.27
+++ sys/arch/amd64/stand/libsa/biosdev.c7 Jun 2016 20:40:16 -
@@ -560,7 +560,7 @@ biosopen(struct open_file *f, ...)
 #endif
 
for (maj = 0; maj < nbdevs &&
-   strncmp(dev, bdevs[maj], devlen); maj++);
+   strncmp(dev, bdevs[maj], devlen); maj++) continue;
if (maj >= nbdevs) {
printf("Unknown device: ");
for (cp = *file; *cp != ':'; cp++)
@@ -671,7 +671,8 @@ biosdisk_err(u_int error)
register const u_char *p = bidos_errs;
 
while (*p && *p != error)
-   while (*p++);
+   while (*p++)
+   continue;
 
return ++p;
 }
@@ -703,7 +704,8 @@ biosdisk_errno(u_int error)
if (error == 0)
return 0;
 
-   for (p = tab; p->error && p->error != error; p++);
+   for (p = tab; p->error && p->error != error; p++)
+   continue;
 
return p->errno;
 }
Index: sys/arch/amd64/stand/libsa/diskprobe.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/diskprobe.c,v
retrieving revision 1.16
diff -u -p -u -r1.16 diskprobe.c
--- sys/arch/amd64/stand/libsa/diskprobe.c  2 Sep 2015 01:52:26 -   
1.16
+++ sys/arch/amd64/stand/libsa/diskprobe.c  7 Jun 2016 20:40:16 -
@@ -266,7 +266,7 @@ diskprobe(void)
 
/* Checksumming of hard disks */
for (i = 0; disksum(i++) && i < MAX_CKSUMLEN; )
-   ;
+   continue;
bios_cksumlen = i;
 
/* Get space for passing bios_diskinfo stuff to kernel */
Index: sys/arch/amd64/stand/libsa/gateA20.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/gateA20.c,v
retrieving revision 1.2
diff -u -p -u -r1.2 gateA20.c
--- sys/arch/amd64/stand/libsa/gateA20.c21 Mar 2004 21:37:41 -  
1.2
+++ sys/arch/amd64/stand/libsa/gateA20.c7 Jun 2016 20:40:16 -
@@ -74,19 +74,19 @@ gateA20(int on)
}
} else {
 
-   while (inb(IO_KBD + KBSTATP) & KBS_IBF);
+   while (inb(IO_KBD + KBSTATP) & KBS_IBF) continue;
 
while (inb(IO_KBD + KBSTATP) & KBS_DIB)
  

Remove unused 'cpuprobe' from i386 bootblocks

[Tom Cosgrove]

Not used, not built, so can just be deleted.  Diff below.

Reduces the diff between i386 and amd64 bootblocks.

Thanks

Tom


Index: sys/arch/i386/stand/libsa/cpuprobe.c
===
RCS file: sys/arch/i386/stand/libsa/cpuprobe.c
diff -N sys/arch/i386/stand/libsa/cpuprobe.c
--- sys/arch/i386/stand/libsa/cpuprobe.c29 Mar 2014 18:09:29 -  
1.2
+++ /dev/null   1 Jan 1970 00:00:00 -
@@ -1,147 +0,0 @@
-/* $OpenBSD: cpuprobe.c,v 1.2 2014/03/29 18:09:29 guenther Exp $   */
-
-/*
- * Copyright (c) 2004 Tom Cosgrove <tom.cosgr...@arches-consulting.com>
- *
- * Permission to use, copy, modify, and distribute this software for any
- * purpose with or without fee is hereby granted, provided that the above
- * copyright notice and this permission notice appear in all copies.
- *
- * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
- * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
- * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
- * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
- * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
- * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
- * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
- */
-
-#include 
-#include 
-
-#include "libsa.h"
-
-int amd64_supported;
-static int cpu_family, cpu_model, cpu_stepping;
-static int psl_check;
-static u_int32_t feature_ecx, feature_edx, feature_amd;
-static char cpu_brandstr[48];  /* Includes term NUL byte */
-static char cpu_vendor[13];/* 12 chars plus NUL term */
-
-/*
- * cpuid instruction.  request in eax, result in eax, ebx, ecx, edx.
- * requires caller to provide u_int32_t regs[4] array.
- */
-u_int32_t
-cpuid(u_int32_t eax, u_int32_t *regs)
-{
-   __asm volatile(
-   "cpuid\n\t"
-   "movl   %%eax, 0(%2)\n\t"
-   "movl   %%ebx, 4(%2)\n\t"
-   "movl   %%ecx, 8(%2)\n\t"
-   "movl   %%edx, 12(%2)\n\t"
-   : "=a" (eax)
-   : "0" (eax), "S" (regs)
-   : "bx", "cx", "dx");
-
-   return eax;
-}
-
-void
-cpuprobe(void)
-{
-   u_int32_t cpuid_max, extended_max;
-   u_int32_t regs[4];
-
-   /*
-* The following is a simple check to see if cpuid is supported.
-* We try to toggle bit 21 (PSL_ID) in eflags.  If it works, then
-* cpuid is supported.  If not, there's no cpuid, and we don't
-* try it (don't want /boot to get an invalid opcode exception).
-*
-* XXX The NexGen Nx586 does not support this bit, so this is not
-* a good method to detect the presence of cpuid on this
-* processor.  That's fine: the purpose here is to detect the
-* absence of cpuid.  We don't mind if the instruction's not
-* there - this is not intended to determine exactly what
-* processor is there, just whether it's i386 or amd64.
-*
-* The only thing that would cause us grief is a processor which
-* does not support cpuid but which does allow the PSL_ID bit
-* in eflags to be toggled.
-*/
-   __asm volatile(
-   "pushfl\n\t"
-   "popl   %2\n\t"
-   "xorl   %2, %0\n\t"
-   "pushl  %0\n\t"
-   "popfl\n\t"
-   "pushfl\n\t"
-   "popl   %0\n\t"
-   "xorl   %2, %0\n\t" /* If %2 == %0, no cpuid */
-   : "=r" (psl_check)
-   : "0" (PSL_ID), "r" (0)
-   : "cc");
-
-   if (psl_check == PSL_ID) {  /* cpuid supported */
-   cpuid_max = cpuid(0, regs); /* Highest std call */
-
-   bcopy([1], cpu_vendor, sizeof(regs[1]));
-   bcopy([3], cpu_vendor + 4, sizeof(regs[3]));
-   bcopy([2], cpu_vendor + 8, sizeof(regs[2]));
-   cpu_vendor[sizeof(cpu_vendor) - 1] = '\0';
-
-   if (cpuid_max >= 1) {
-   u_int32_t id;
-
-   id = cpuid(1, regs);/* Get basic info */
-   cpu_stepping = id & 0x00f;
-   cpu_model = (id >> 4) & 0x000f;
-   cpu_family = (id >> 8) & 0x000f;
-
-   feature_ecx = regs[2];
-   feature_edx = regs[3];
-   }
-
-   extended_max = cpuid(0x8000, regs); /* Highest ext  */
-
-   if (extended_max >= 0x8001) {
-  

'continue' to appease style gods in i386,amd64 libsa

[Tom Cosgrove]

As per subject, a couple of empty loop bodies in the i396 and amd64 boot blocks.

Diff below.

Tom


Index: sys/arch/amd64/stand/libsa/biosdev.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/biosdev.c,v
retrieving revision 1.27
diff -u -p -u -r1.27 biosdev.c
--- sys/arch/amd64/stand/libsa/biosdev.c1 Oct 2015 20:28:12 -   
1.27
+++ sys/arch/amd64/stand/libsa/biosdev.c6 Jun 2016 21:04:07 -
@@ -671,7 +671,8 @@ biosdisk_err(u_int error)
register const u_char *p = bidos_errs;
 
while (*p && *p != error)
-   while (*p++);
+   while (*p++)
+   continue;
 
return ++p;
 }
@@ -703,7 +704,8 @@ biosdisk_errno(u_int error)
if (error == 0)
return 0;
 
-   for (p = tab; p->error && p->error != error; p++);
+   for (p = tab; p->error && p->error != error; p++)
+   continue;
 
return p->errno;
 }
Index: sys/arch/i386/stand/libsa/biosdev.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/i386/stand/libsa/biosdev.c,v
retrieving revision 1.92
diff -u -p -u -r1.92 biosdev.c
--- sys/arch/i386/stand/libsa/biosdev.c 1 Oct 2015 20:28:12 -   1.92
+++ sys/arch/i386/stand/libsa/biosdev.c 6 Jun 2016 21:04:07 -
@@ -672,7 +672,8 @@ biosdisk_err(u_int error)
register const u_char *p = bidos_errs;
 
while (*p && *p != error)
-   while (*p++);
+   while (*p++)
+   continue;
 
return ++p;
 }
@@ -704,7 +705,8 @@ biosdisk_errno(u_int error)
if (error == 0)
return 0;
 
-   for (p = tab; p->error && p->error != error; p++);
+   for (p = tab; p->error && p->error != error; p++)
+   continue;
 
return p->errno;
 }