Date: Tue, 11 Feb 2025 18:10:23 +0800 From: Zhenlei Huang <zlei@FreeBSD.org> To: Jessica Clarke <jrtc27@freebsd.org> Cc: Doug Moore <dougm@FreeBSD.org>, "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-branches@freebsd.org" <dev-commits-src-branches@FreeBSD.org> Subject: Re: git: 7bcc7a0b88cc - stable/14 - libkern: avoid local var in order_base_2() Message-ID: <5BF2F37C-16B4-4B39-B14B-131CE430BB4A@FreeBSD.org> In-Reply-To: <803D3DBC-A4F5-47FB-8034-5241461D4A71@freebsd.org> References: <202502101115.51ABFISj016298@gitrepo.freebsd.org> <858BD10B-A2C9-4D3A-9FF5-C5573597DF8B@FreeBSD.org> <803D3DBC-A4F5-47FB-8034-5241461D4A71@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--Apple-Mail=_578F92E7-3BA6-4432-8E6C-74615E94D42A Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On Feb 11, 2025, at 1:23 PM, Jessica Clarke <jrtc27@freebsd.org> = wrote: >=20 > On 11 Feb 2025, at 03:30, Zhenlei Huang <zlei@FreeBSD.org = <mailto:zlei@FreeBSD.org>> wrote: >>> On Feb 10, 2025, at 7:15 PM, Doug Moore <dougm@FreeBSD.org> wrote: >>>=20 >>> The branch stable/14 has been updated by dougm: >>>=20 >>> URL: = https://cgit.FreeBSD.org/src/commit/?id=3D7bcc7a0b88ccb5e1fe31de88ab9a17e4= 6859318b >>>=20 >>> commit 7bcc7a0b88ccb5e1fe31de88ab9a17e46859318b >>> Author: Doug Moore <dougm@FreeBSD.org> >>> AuthorDate: 2024-09-27 23:43:07 +0000 >>> Commit: Doug Moore <dougm@FreeBSD.org> >>> CommitDate: 2025-02-10 10:30:05 +0000 >>>=20 >>> libkern: avoid local var in order_base_2() >>>=20 >>> order_base_2(n) is implemented with a variable, which keeps it from >>> being used at file scope. Implement it instead as ilog2(2*n-1), = which >>> produces a different result when 2*n overflows, which appears = unlikely >>> in practice. >>>=20 >>> Reviewed by: bz >>> Differential Revision: https://reviews.freebsd.org/D46826 >>>=20 >>> (cherry picked from commit = b7cbf741d55468ba34305a14ac3acc1c286af034) >>> --- >>> sys/sys/libkern.h | 5 +---- >>> 1 file changed, 1 insertion(+), 4 deletions(-) >>>=20 >>> diff --git a/sys/sys/libkern.h b/sys/sys/libkern.h >>> index a10289d72dc7..835e5ffaf0b7 100644 >>> --- a/sys/sys/libkern.h >>> +++ b/sys/sys/libkern.h >>> @@ -229,10 +229,7 @@ ilog2_long_long(long long n) >>>=20 >>> #define ilog2(n) (__builtin_constant_p(n) ? ilog2_const(n) : = ilog2_var(n)) >>> #define rounddown_pow_of_two(n) ((__typeof(n))1 << ilog2(n)) >>> -#define order_base_2(n) ({ \ >>> - __typeof(n) _n =3D (n); \ >>=20 >> This local var `_n` is within the scope of the macro `order_base_2`, = it is surrounded with >> {} and is harmless, so it will not pollute the file scoped variables. >>=20 >> Am I reading the commit message wrong ? >=20 > It=E2=80=99s not about pollution. GNU statement expressions are just = not valid > at file scope, they must be used within a function (since otherwise = the > implementation would have to have a full blown interpreter to constant > evaluate the whole thing, just like C++ and now C to some extent have > ended up with these days with constexpr). >=20 > That is, you cannot write: >=20 > int x =3D ({ ... }); >=20 > at file scope. Ahh, I see. Thanks for the explaining! >=20 > Jess --Apple-Mail=_578F92E7-3BA6-4432-8E6C-74615E94D42A Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 <html><head><meta http-equiv=3D"Content-Type" content=3D"text/html; = charset=3Dutf-8"></head><body style=3D"word-wrap: break-word; = -webkit-nbsp-mode: space; line-break: after-white-space;" class=3D""><br = class=3D""><div><br class=3D""><blockquote type=3D"cite" class=3D""><div = class=3D"">On Feb 11, 2025, at 1:23 PM, Jessica Clarke <<a = href=3D"mailto:jrtc27@freebsd.org" class=3D"">jrtc27@freebsd.org</a>> = wrote:</div><br class=3D"Apple-interchange-newline"><div class=3D""><meta = charset=3D"UTF-8" class=3D""><span style=3D"caret-color: rgb(0, 0, 0); = font-family: Menlo-Regular; font-size: 13px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none; float: none; display: inline !important;" = class=3D"">On 11 Feb 2025, at 03:30, Zhenlei Huang <</span><a = href=3D"mailto:zlei@FreeBSD.org" style=3D"font-family: Menlo-Regular; = font-size: 13px; font-style: normal; font-variant-caps: normal; = font-weight: 400; letter-spacing: normal; orphans: auto; text-align: = start; text-indent: 0px; text-transform: none; white-space: normal; = widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; = -webkit-text-stroke-width: 0px;" class=3D"">zlei@FreeBSD.org</a><span = style=3D"caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; = font-size: 13px; font-style: normal; font-variant-caps: normal; = font-weight: 400; letter-spacing: normal; text-align: start; = text-indent: 0px; text-transform: none; white-space: normal; = word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: = none; float: none; display: inline !important;" class=3D"">> = wrote:</span><br style=3D"caret-color: rgb(0, 0, 0); font-family: = Menlo-Regular; font-size: 13px; font-style: normal; font-variant-caps: = normal; font-weight: 400; letter-spacing: normal; text-align: start; = text-indent: 0px; text-transform: none; white-space: normal; = word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: = none;" class=3D""><blockquote type=3D"cite" style=3D"font-family: = Menlo-Regular; font-size: 13px; font-style: normal; font-variant-caps: = normal; font-weight: 400; letter-spacing: normal; orphans: auto; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; = -webkit-text-stroke-width: 0px; text-decoration: none;" = class=3D""><blockquote type=3D"cite" class=3D"">On Feb 10, 2025, at 7:15 = PM, Doug Moore <<a href=3D"mailto:dougm@FreeBSD.org" = class=3D"">dougm@FreeBSD.org</a>> wrote:<br class=3D""><br = class=3D"">The branch stable/14 has been updated by dougm:<br = class=3D""><br class=3D"">URL: <a = href=3D"https://cgit.FreeBSD.org/src/commit/?id=3D7bcc7a0b88ccb5e1fe31de88= ab9a17e46859318b" = class=3D"">https://cgit.FreeBSD.org/src/commit/?id=3D7bcc7a0b88ccb5e1fe31d= e88ab9a17e46859318b</a><br class=3D""><br class=3D"">commit = 7bcc7a0b88ccb5e1fe31de88ab9a17e46859318b<br class=3D"">Author: = Doug Moore <<a = href=3D"mailto:dougm@FreeBSD.org" class=3D"">dougm@FreeBSD.org</a>><br = class=3D"">AuthorDate: 2024-09-27 23:43:07 +0000<br class=3D"">Commit: = Doug Moore <<a = href=3D"mailto:dougm@FreeBSD.org" class=3D"">dougm@FreeBSD.org</a>><br = class=3D"">CommitDate: 2025-02-10 10:30:05 +0000<br class=3D""><br = class=3D""> libkern: avoid local var in order_base_2()<br = class=3D""><br class=3D""> order_base_2(n) is implemented with a = variable, which keeps it from<br class=3D""> being used at file = scope. Implement it instead as ilog2(2*n-1), which<br = class=3D""> produces a different result when 2*n overflows, which = appears unlikely<br class=3D""> in practice.<br class=3D""><br = class=3D""> Reviewed by: bz<br = class=3D""> Differential Revision: <a = href=3D"https://reviews.freebsd.org/D46826" = class=3D"">https://reviews.freebsd.org/D46826</a><br class=3D""><br = class=3D""> (cherry picked from commit = b7cbf741d55468ba34305a14ac3acc1c286af034)<br class=3D"">---<br = class=3D"">sys/sys/libkern.h | 5 +----<br class=3D"">1 file changed, 1 = insertion(+), 4 deletions(-)<br class=3D""><br class=3D"">diff --git = a/sys/sys/libkern.h b/sys/sys/libkern.h<br class=3D"">index = a10289d72dc7..835e5ffaf0b7 100644<br class=3D"">--- = a/sys/sys/libkern.h<br class=3D"">+++ b/sys/sys/libkern.h<br class=3D"">@@= -229,10 +229,7 @@ ilog2_long_long(long long n)<br class=3D""><br = class=3D"">#define ilog2(n) (__builtin_constant_p(n) ? ilog2_const(n) : = ilog2_var(n))<br class=3D"">#define rounddown_pow_of_two(n) = ((__typeof(n))1 << ilog2(n))<br class=3D"">-#define = order_base_2(n) ({ \<br class=3D"">- __typeof(n) _n =3D (n); \<br = class=3D""></blockquote><br class=3D"">This local var `_n` is within the = scope of the macro `order_base_2`, it is surrounded with<br class=3D"">{} = and is harmless, so it will not pollute the file scoped variables.<br = class=3D""><br class=3D"">Am I reading the commit message wrong ?<br = class=3D""></blockquote><br style=3D"caret-color: rgb(0, 0, 0); = font-family: Menlo-Regular; font-size: 13px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none;" class=3D""><span style=3D"caret-color: rgb(0, 0, = 0); font-family: Menlo-Regular; font-size: 13px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none; float: none; display: inline !important;" = class=3D"">It=E2=80=99s not about pollution. GNU statement expressions = are just not valid</span><br style=3D"caret-color: rgb(0, 0, 0); = font-family: Menlo-Regular; font-size: 13px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none;" class=3D""><span style=3D"caret-color: rgb(0, 0, = 0); font-family: Menlo-Regular; font-size: 13px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none; float: none; display: inline !important;" = class=3D"">at file scope, they must be used within a function (since = otherwise the</span><br style=3D"caret-color: rgb(0, 0, 0); font-family: = Menlo-Regular; font-size: 13px; font-style: normal; font-variant-caps: = normal; font-weight: 400; letter-spacing: normal; text-align: start; = text-indent: 0px; text-transform: none; white-space: normal; = word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: = none;" class=3D""><span style=3D"caret-color: rgb(0, 0, 0); font-family: = Menlo-Regular; font-size: 13px; font-style: normal; font-variant-caps: = normal; font-weight: 400; letter-spacing: normal; text-align: start; = text-indent: 0px; text-transform: none; white-space: normal; = word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: = none; float: none; display: inline !important;" class=3D"">implementation = would have to have a full blown interpreter to constant</span><br = style=3D"caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; = font-size: 13px; font-style: normal; font-variant-caps: normal; = font-weight: 400; letter-spacing: normal; text-align: start; = text-indent: 0px; text-transform: none; white-space: normal; = word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: = none;" class=3D""><span style=3D"caret-color: rgb(0, 0, 0); font-family: = Menlo-Regular; font-size: 13px; font-style: normal; font-variant-caps: = normal; font-weight: 400; letter-spacing: normal; text-align: start; = text-indent: 0px; text-transform: none; white-space: normal; = word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: = none; float: none; display: inline !important;" class=3D"">evaluate the = whole thing, just like C++ and now C to some extent have</span><br = style=3D"caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; = font-size: 13px; font-style: normal; font-variant-caps: normal; = font-weight: 400; letter-spacing: normal; text-align: start; = text-indent: 0px; text-transform: none; white-space: normal; = word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: = none;" class=3D""><span style=3D"caret-color: rgb(0, 0, 0); font-family: = Menlo-Regular; font-size: 13px; font-style: normal; font-variant-caps: = normal; font-weight: 400; letter-spacing: normal; text-align: start; = text-indent: 0px; text-transform: none; white-space: normal; = word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: = none; float: none; display: inline !important;" class=3D"">ended up with = these days with constexpr).</span><br style=3D"caret-color: rgb(0, 0, = 0); font-family: Menlo-Regular; font-size: 13px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none;" class=3D""><br style=3D"caret-color: rgb(0, 0, = 0); font-family: Menlo-Regular; font-size: 13px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none;" class=3D""><span style=3D"caret-color: rgb(0, 0, = 0); font-family: Menlo-Regular; font-size: 13px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none; float: none; display: inline !important;" = class=3D"">That is, you cannot write:</span><br style=3D"caret-color: = rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 13px; font-style: = normal; font-variant-caps: normal; font-weight: 400; letter-spacing: = normal; text-align: start; text-indent: 0px; text-transform: none; = white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none;" class=3D""><br style=3D"caret-color: rgb(0, 0, = 0); font-family: Menlo-Regular; font-size: 13px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none;" class=3D""><span style=3D"caret-color: rgb(0, 0, = 0); font-family: Menlo-Regular; font-size: 13px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none; float: none; display: inline !important;" = class=3D""> int x =3D ({ ... });</span><br style=3D"caret-color: = rgb(0, 0, 0); font-family: Menlo-Regular; font-size: 13px; font-style: = normal; font-variant-caps: normal; font-weight: 400; letter-spacing: = normal; text-align: start; text-indent: 0px; text-transform: none; = white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none;" class=3D""><br style=3D"caret-color: rgb(0, 0, = 0); font-family: Menlo-Regular; font-size: 13px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none;" class=3D""><span style=3D"caret-color: rgb(0, 0, = 0); font-family: Menlo-Regular; font-size: 13px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none; float: none; display: inline !important;" = class=3D"">at file scope.</span><br style=3D"caret-color: rgb(0, 0, 0); = font-family: Menlo-Regular; font-size: 13px; font-style: normal; = font-variant-caps: normal; font-weight: 400; letter-spacing: normal; = text-align: start; text-indent: 0px; text-transform: none; white-space: = normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; = text-decoration: none;" class=3D""></div></blockquote><div><br = class=3D""></div><div>Ahh, I see. Thanks for the explaining!</div><br = class=3D""><blockquote type=3D"cite" class=3D""><div class=3D""><br = style=3D"caret-color: rgb(0, 0, 0); font-family: Menlo-Regular; = font-size: 13px; font-style: normal; font-variant-caps: normal; = font-weight: 400; letter-spacing: normal; text-align: start; = text-indent: 0px; text-transform: none; white-space: normal; = word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: = none;" class=3D""><span style=3D"caret-color: rgb(0, 0, 0); font-family: = Menlo-Regular; font-size: 13px; font-style: normal; font-variant-caps: = normal; font-weight: 400; letter-spacing: normal; text-align: start; = text-indent: 0px; text-transform: none; white-space: normal; = word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: = none; float: none; display: inline !important;" = class=3D"">Jess</span></div></blockquote></div><br class=3D""><div = class=3D""> <div><br class=3D""></div> </div> <br class=3D""></body></html>= --Apple-Mail=_578F92E7-3BA6-4432-8E6C-74615E94D42A--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5BF2F37C-16B4-4B39-B14B-131CE430BB4A>