Date: Mon, 03 Nov 2014 00:34:15 -0800 From: Xin Li <delphij@delphij.net> To: Bruce Evans <brde@optusnet.com.au>, d@delphij.net Cc: "src-committers@freebsd.org" <src-committers@freebsd.org>, Ian Lepore <ian@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, Mark R V Murray <mark@grondar.org>, Konstantin Belousov <kostikbel@gmail.com> Subject: Re: svn commit: r273958 - head/sys/dev/random Message-ID: <54573E07.5040505@delphij.net> In-Reply-To: <20141103115402.H3149@besplex.bde.org> References: <201411020201.sA221unt091493@svn.freebsd.org> <720EB74E-094A-43F3-8B1C-47BC7F6FECC3@grondar.org> <1414934579.17308.248.camel@revolution.hippie.lan> <6FB65828-6A79-4BDE-A9F7-BC472BA538CE@grondar.org> <CAJ-VmomeOwE3LOpehhJ__G=FCoBDRXrrn%2BSfjwPFODts6YYHNQ@mail.gmail.com> <20141102192057.GB53947@kib.kiev.ua> <29A795E1-19E2-49E4-9653-143D3F6F3F12@grondar.org> <20141102194625.GC53947@kib.kiev.ua> <751CD860-95B9-4F68-AE69-976B42823AD0@grondar.org> <54568E41.8030305@delphij.net> <20141102201331.GE53947@kib.kiev.ua> <545693B4.8030602@delphij.net> <1414961583.1200.27.camel@revolution.hippie.lan> <5456A47E.2030405@delphij.net> <5456A69C.6040902@delphij.net> <20141103115402.H3149@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------070009050000080809030508
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
On 11/2/14 5:13 PM, Bruce Evans wrote:
> % - % - KASSERT(random_adaptor != NULL, ("No active random
> adaptor in %s", __func__)); % } % % void
>
> Lots of style bugs (long lines, use of the __func__ obfuscation,
> and general verboseness).
What would you recommend doing for cases like this? Do we just do
KASSERT without the __func__ and rely on ddb to provide the
information in backtrace or something else?
Speaking for long lines -- I usually use the MTA's default so the diff
won't wrap. style(9) is a little bit vague here, what should new code
appear like?
> % @@ -256,11 +258,15 @@ random_adaptor_read(struct cdev *dev
> __unused, str % /* Let the entropy source do any post-read
> cleanup. */ % (random_adaptor->ra_read)(NULL, 1); % % -
> free(random_buf, M_ENTROPY); % + if (nbytes !=
> uio->uio_resid && (error == ERESTART || % + error ==
> EINTR) ) % + error = 0; /* Return partial read, not
> error. */
>
> This adjustment has no effect. Upper layers already do it for
> these 2 errors and one more. Other cases remain broken.
>
> It is simpler to do nothing. Let upper layers get it right or
> wrong. Doesn't matter much either way.
Will remove these two portions of the change.
> % +
>
> More style bugs. Extra blank line here. Space bereofe parentheses
> above.
>
> % } % - % sx_sunlock(&random_adaptors_lock); % % +
> free(random_buf, M_ENTROPY); % +
>
> Extra blank lines are sprinkled randomly. Mostly added, but one
> was removed. There is an extra blank line after the sx_slock() that
> separates it from the blocks(s) of code that it locks; the removed
> one used the same style.
Will change the code to consistently use no blank line after
sx_slock() and before sx_sunlock() if that's correct, can you confirm?
Cheers,
-----BEGIN PGP SIGNATURE-----
iQIcBAEBCgAGBQJUVz4HAAoJEJW2GBstM+nsZnkP/0PDU6SvwEG94w/csmWiWRuE
IhhmH3oKz+LAzsU7pGUPpyE+UqWaMhRod6CHH9Io7EaJmZ9gsoMD29KWSdrYhWmS
eMuZIt/vNkHGd+4SpFMUVq15s1gmRO2AK7wev6PZ1lOeqlJQtEkelnwaO7gWZdyp
yWjdBW3MOOuVPR/kj0qF7Nx3nuCPkixO/SldXbMOeM0ofKhZw2DkiJmjSDx17Mk7
q/x2cuui4+nDxyJleFeLDpZqI+zaII/2yGly8OMzmhOsaAUtQrhxFM5I8Py/E85t
yIK6U01tuRu1pVMNtqY1EgFqEkzRAr/kWG/wHZOuiITNroGye5J+u9VLaBVgCmQF
hJW2Wthq2qFaWX2oqlSamHSss1Mt048iIzIzQd7GhwRYb6Be5IGPaqI2QOyKAYeE
ojHvddkL30xIVLwM9qkFmIiyOvaC4gi/dSc6cbSEEYarTLqsIUZlJ1Xx5X5OK9y9
sAsIIo+0v4njB/cv9x4e5aeDXsRU1ABFHk4IUb7hBv2/Um9FRTuVKyKDWiEm4mRV
U/uDxRDIFc0KAFj0bEtJXyeB/GsGsYqlba63Z978QQUvtlk7Rjtq8hDBks+I6CcC
Hb8UOttzY+jS/XDxphNw5eXeg6rgy0v6isLqSAJvmwnyrXCUPJF5oMYxUla3lYrc
QzxqUTKnPyyid4Hl+7n9
=ixEW
-----END PGP SIGNATURE-----
--------------070009050000080809030508
Content-Type: text/x-patch;
name="random-style.diff"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
filename="random-style.diff"
Index: sys/dev/random/random_adaptors.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- sys/dev/random/random_adaptors.c (revision 274006)
+++ sys/dev/random/random_adaptors.c (working copy)
@@ -210,7 +210,6 @@ random_adaptor_read(struct cdev *dev __unused, str
random_buf =3D malloc(PAGE_SIZE, M_ENTROPY, M_WAITOK);
=20
sx_slock(&random_adaptors_lock);
-
KASSERT(random_adaptor !=3D NULL, ("No active random adaptor in %s", __=
func__));
=20
/* Let the entropy source do any pre-read setup. */
@@ -234,15 +233,15 @@ random_adaptor_read(struct cdev *dev __unused, str
(random_adaptor->ra_read)(NULL, 0);
}
=20
+ /*
+ * The read-rate stuff is a rough indication of the instantaneous
+ * read rate, used to increase the use of 'live' entropy sources
+ * when lots of reads are done.
+ */
mtx_lock(&random_read_rate_mtx);
-
- /* The read-rate stuff is a rough indication of the instantaneous read =
rate,
- * used to increase the use of 'live' entropy sources when lots of read=
s are done.
- */
nbytes =3D (uio->uio_resid + 32 - 1)/32; /* Round up to units of 32 */
random_adaptor_read_rate_cache +=3D nbytes*32;
random_adaptor_read_rate_cache =3D MIN(random_adaptor_read_rate_cache, =
32);
-
mtx_unlock(&random_read_rate_mtx);
=20
if (error =3D=3D 0) {
@@ -257,11 +256,6 @@ random_adaptor_read(struct cdev *dev __unused, str
=20
/* Let the entropy source do any post-read cleanup. */
(random_adaptor->ra_read)(NULL, 1);
-
- if (nbytes !=3D uio->uio_resid && (error =3D=3D ERESTART ||
- error =3D=3D EINTR) )
- error =3D 0; /* Return partial read, not error. */
-
}
sx_sunlock(&random_adaptors_lock);
=20
@@ -296,11 +290,9 @@ random_adaptor_write(struct cdev *dev __unused, st
#ifdef RANDOM_DEBUG
printf("random: %s %zd\n", __func__, uio->uio_resid);
#endif
-
random_buf =3D malloc(PAGE_SIZE, M_ENTROPY, M_WAITOK);
=20
sx_slock(&random_adaptors_lock);
-
KASSERT(random_adaptor !=3D NULL, ("No active random adaptor in %s", __=
func__));
=20
nbytes =3D uio->uio_resid;
@@ -317,15 +309,9 @@ random_adaptor_write(struct cdev *dev __unused, st
KASSERT(random_adaptor !=3D NULL, ("No active random adaptor in %s",
__func__));
}
-
sx_sunlock(&random_adaptors_lock);
=20
- if (nbytes !=3D uio->uio_resid && (error =3D=3D ERESTART ||
- error =3D=3D EINTR) )
- error =3D 0; /* Partial write, not error. */
-
free(random_buf, M_ENTROPY);
-
return (error);
}
=20
@@ -339,7 +325,6 @@ random_adaptor_poll(struct cdev *dev __unused, int
#endif
=20
sx_slock(&random_adaptors_lock);
-
KASSERT(random_adaptor !=3D NULL, ("No active random adaptor in %s", __=
func__));
=20
if (events & (POLLIN | POLLRDNORM)) {
@@ -348,7 +333,6 @@ random_adaptor_poll(struct cdev *dev __unused, int
else
selrecord(td, &rsel);
}
-
sx_sunlock(&random_adaptors_lock);
=20
return (events);
--------------070009050000080809030508--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?54573E07.5040505>