From nobody Wed Jul 05 15:50:53 2023 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Qx3zM5Z7Cz4lPVr; Wed, 5 Jul 2023 15:50:55 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-oo1-xc36.google.com (mail-oo1-xc36.google.com [IPv6:2607:f8b0:4864:20::c36]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Qx3zM3rmNz4GHg; Wed, 5 Jul 2023 15:50:55 +0000 (UTC) (envelope-from mjguzik@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-oo1-xc36.google.com with SMTP id 006d021491bc7-56598263d1dso4378044eaf.0; Wed, 05 Jul 2023 08:50:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1688572254; x=1691164254; h=content-transfer-encoding:cc:to:subject:message-id:date:from :references:in-reply-to:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=9gsg7IgfkA6ZNW7nO0fOmoEiP5BlnzVO4lATFdRAMRI=; b=ZO0BnBkrMFLN6SxG456NRFBmJ3GqTS457W5dMS4x/kePK4PMs4r04vWV+rrpwJH66p 2dd8X/vOROrS/pbW8oocSY7g9giC0n2LLHA9X0VvDfevkpyYU9hTey45VTTof/K2sty1 gBA+9aZ11wIRMO1g1XTIfjGDOElaUe6PQPiPg2IdK4itRxV+exv5fXt6/9HeQCToWZ+V oKAI6zoDyN3d6Wo56/0SQVakw4IqhAul9mMsZhFziNFvtg5Bmi1M5QX1AtWwUdx9GVE8 o3XU+6hDLQX6cfKmuVsGwF7e+YC67KSii2ldwwF/u0SFz/IcB3sB/Gs9wgAfaAUGZeCI AZqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688572254; x=1691164254; h=content-transfer-encoding:cc:to:subject:message-id:date:from :references:in-reply-to:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=9gsg7IgfkA6ZNW7nO0fOmoEiP5BlnzVO4lATFdRAMRI=; b=Sb6Nu+eeJ5LkobVx6SjYcGNRWcJcW+AczyEllUIfgOeQ7NwmP4WA//CfV0Cn5W4Kal o0Fz154lkt7CqJJYilKhaxGYfjglUoK04rWREwRUPv5HnPbEeZMsYO/UFGNMD1afkwri z4/MTQ1j+h14WvdRIC85rMsrBOmbPzrhGU9JDdtCHsh2Bn+uUkfdM+TapPqP80KkDjwf jBAV+CYe3aMyynnnAlQLyhZkiOE0uPfJa/71TE+4wdag5nRM5kYuVWE6G0bJbTHmqjVu doP8sOw2VtXcNck3r8MMuim00VYJbDBaaUtUr526w0ETkGBbnbHQJnkYZosOSI6tYVq/ /rNQ== X-Gm-Message-State: AC+VfDyCjIPwlVe94icHCrrTTqPvpTq3Xsds5lNu4hqXWJ7LewG00Mje kg9Xch5N94fYdrEv29wRf/wikXTsd96/3At697adpe3Y X-Google-Smtp-Source: ACHHUZ5pJ8CUPwcMKcCFIegmYd8Cx4iAbAcHFBnGaVXLc5Jop/st5k3vK43FusUTjOAuCfchOM1/sJPHKDzyFoNleJU= X-Received: by 2002:a4a:5805:0:b0:565:32bc:2166 with SMTP id f5-20020a4a5805000000b0056532bc2166mr11682959oob.9.1688572253846; Wed, 05 Jul 2023 08:50:53 -0700 (PDT) List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Received: by 2002:a8a:1109:0:b0:4e1:6042:cdc9 with HTTP; Wed, 5 Jul 2023 08:50:53 -0700 (PDT) In-Reply-To: <0AD6BA4D-E6B1-4C56-B7B7-A85D57188A34@freebsd.org> References: <202307051538.365FcoZR017983@gitrepo.freebsd.org> <0AD6BA4D-E6B1-4C56-B7B7-A85D57188A34@freebsd.org> From: Mateusz Guzik Date: Wed, 5 Jul 2023 17:50:53 +0200 Message-ID: Subject: Re: git: cebb8646c4ee - main - Support byte-sized enums To: Jessica Clarke Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 4Qx3zM3rmNz4GHg X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N On 7/5/23, Jessica Clarke wrote: > On 5 Jul 2023, at 16:38, Mateusz Guzik wrote: >> >> The branch main has been updated by mjg: >> >> URL: >> https://cgit.FreeBSD.org/src/commit/?id=3Dcebb8646c4ee559aedcbc1b27bf30f= aa70a1f716 >> >> commit cebb8646c4ee559aedcbc1b27bf30faa70a1f716 >> Author: Mateusz Guzik >> AuthorDate: 2023-03-12 19:29:20 +0000 >> Commit: Mateusz Guzik >> CommitDate: 2023-07-05 14:46:30 +0000 >> >> Support byte-sized enums >> >> To that end add __enum_uint8_decl and __enum_uint8. >> >> By default enums take 4 bytes, but vast majority of them have values >> which would fit in a byte. >> >> One can try to workaround the problem by using bitfields, like so: >> enum some_small_enum foo:8; >> >> but that's ugly and runs into trouble when using atomic_load (you >> can't >> take an address of a bitfield, even if it is sized to a multiply of a >> byte). >> >> Both gcc 13 and clang support specifying the size, and for older >> variants one can fallback to the "packed" attribute. >> >> Names are mangled in order to avoid mix use with plain enum. >> >> Reviewed by: >> Differential Revision: https://reviews.freebsd.org/D39031 >> --- >> sys/sys/types.h | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/sys/sys/types.h b/sys/sys/types.h >> index 0846f2a57670..c6f7abe1f524 100644 >> --- a/sys/sys/types.h >> +++ b/sys/sys/types.h >> @@ -347,6 +347,18 @@ __makedev(int _Major, int _Minor) >> ((dev_t)(_Minor & 0xff00) << 24) | (_Minor & 0xffff00ff)); >> } >> >> +#if (defined(__clang__) || (defined(__GNUC__) && __GNUC__ >=3D 13)) >> +#define __enum_uint8_decl(name) enum enum_ ## name ## _uint8 : uint8_t >> +#define __enum_uint8(name) enum enum_ ## name ## _uint8 > > What happened to zlei@=E2=80=99s feedback about making it so __enum_uint8= isn=E2=80=99t > needed everywhere? > The feedback got addressed before it got issued, both in the commit message (pasted in the review) and my earlier comment. It explicitly, albeit tersely, states the point was to disallow accidental size mismatch. Here is a trivial I ran into: there was struct xvnode containing 'enum vtype vtype;' which was exported to userspace. Changing the size of the enum changed the size in struct vnode (as excpected) and uintentionally changed the size of the field in that struct xvnode as well and there was no indication of it at compilation time. (Turns out the struct was stale so it got whacked altogether btw.) Making sure a resized enum is only accessible with dedicated naming prevents the above kind of a problem from occurring and makes it clear for all consumers what size they are getting. > >> +#else >> +/* >> + * Note: there is no real size checking here, but the code below can be >> + * removed once we require GCC 13. >> + */ >> +#define __enum_uint8_decl(name) enum __attribute__((packed)) enum_ ## >> name ## _uint8 >> +#define __enum_uint8(name) enum __attribute__((packed)) enum_ ## name #= # >> _uint8 >> +#endif >> + >> /* >> * These declarations belong elsewhere, but are repeated here and in >> * to give broken programs a better chance of working with > > --=20 Mateusz Guzik