Re: CVS commit: src/sys/arch
> On Jan 25, 2021, at 9:45 AM, Robert Elz wrote:
>
>Date:Mon, 25 Jan 2021 08:19:44 -0800
>From:Jason Thorpe
>Message-ID:
>
> | Using { 0 } makes an assumption about the first member of the
> | structure which is not guaranteed to remain true.
>
> That's right, but you could explicitly init a named field, most likely
> the one that is tested to determine that this is the sentinel, eg: from
> one of the recent updates ...
>
> static const struct device_compatible_entry compat_data[] = {
>{ .compat = "pnpPNP,401" },
> - { 0 }
> + { }
> };
>
> that could instead be changed to
> { .compat = NULL }
>
> (or something similar to that).
I noticed this because of a different local change in my tree that makes the
first field another anonymous union.
Anyhow, I'll go ahead and define a standard sentinel macro that can be used for
the common { .compat = XXX } case, and fire up sed to fix up the tree.
-- thorpej
Re: CVS commit: src/sys/arch
On 25.01.2021 17:19, Jason Thorpe wrote:
>
>> On Jan 25, 2021, at 6:22 AM, Kamil Rytarowski wrote:
>>
>> I have no problem with this change but I am curious why should we use "{
>> }"? It's a C GNU extension and C++ syntax.
>
> Using { 0 } makes an assumption about the first member of the structure which
> is not guaranteed to remain true.
>
Both versions should work in the same way, except that {0} is the
standard variation and {} an extension.
I have got no preference for either.
> -- thorpej
>
signature.asc
Description: OpenPGP digital signature
Re: CVS commit: src/sys/arch
On Mon, Jan 25, 2021 at 20:28:52 +0300, Valery Ushakov wrote:
> On Mon, Jan 25, 2021 at 08:19:44 -0800, Jason Thorpe wrote:
>
> > > On Jan 25, 2021, at 6:22 AM, Kamil Rytarowski wrote:
> > >
> > > I have no problem with this change but I am curious why should we use "{
> > > }"? It's a C GNU extension and C++ syntax.
> >
> > Using { 0 } makes an assumption about the first member of the
> > structure which is not guaranteed to remain true.
>
> The commit message says:
>
> | Since we're using designated initialisers for compat data, we should
> | use a completely empty initializer for the sentinel.
>
> but that "should" is not true. The code that checks that sentinel
> uses some particular member to access it, so, pedantically speaking,
> the initialization must designate that member in the sentinel
> initialization. Yes, this is verbose and doesn't look as pretty, but
> that is what "should" happen here. Using non-standard {} extension
> makes it look nicer, but is not a "should" kind of necessity.
PS: Forgot to add that C++ doesn't have designated initializers (or at
least it didn't last time I looked), so they are in a different
situation here and need an empty initializer list. In C the only
difference it makes is, as far as I can tell, exactly this kind of an
array with a sentinel at the end and the difference is cosmetic.
-uwe
Re: CVS commit: src/sys/arch
Date:Mon, 25 Jan 2021 08:19:44 -0800
From:Jason Thorpe
Message-ID:
| Using { 0 } makes an assumption about the first member of the
| structure which is not guaranteed to remain true.
That's right, but you could explicitly init a named field, most likely
the one that is tested to determine that this is the sentinel, eg: from
one of the recent updates ...
static const struct device_compatible_entry compat_data[] = {
{ .compat = "pnpPNP,401" },
- { 0 }
+ { }
};
that could instead be changed to
{ .compat = NULL }
(or something similar to that).
kre
Re: CVS commit: src/sys/arch
On Mon, Jan 25, 2021 at 08:19:44 -0800, Jason Thorpe wrote:
> > On Jan 25, 2021, at 6:22 AM, Kamil Rytarowski wrote:
> >
> > I have no problem with this change but I am curious why should we use "{
> > }"? It's a C GNU extension and C++ syntax.
>
> Using { 0 } makes an assumption about the first member of the
> structure which is not guaranteed to remain true.
The commit message says:
| Since we're using designated initialisers for compat data, we should
| use a completely empty initializer for the sentinel.
but that "should" is not true. The code that checks that sentinel
uses some particular member to access it, so, pedantically speaking,
the initialization must designate that member in the sentinel
initialization. Yes, this is verbose and doesn't look as pretty, but
that is what "should" happen here. Using non-standard {} extension
makes it look nicer, but is not a "should" kind of necessity.
-uwe
Re: CVS commit: src/sys/arch
> On Jan 25, 2021, at 6:22 AM, Kamil Rytarowski wrote:
>
> I have no problem with this change but I am curious why should we use "{
> }"? It's a C GNU extension and C++ syntax.
Using { 0 } makes an assumption about the first member of the structure which
is not guaranteed to remain true.
-- thorpej
Re: CVS commit: src/sys/arch
On 25.01.2021 15:20, Jason R Thorpe wrote:
> Module Name: src
> Committed By: thorpej
> Date: Mon Jan 25 14:20:39 UTC 2021
>
> Modified Files:
> src/sys/arch/arm/altera: cycv_clkmgr.c
> src/sys/arch/arm/amlogic: meson_pinctrl.c meson_pwm.c meson_thermal.c
> meson_usbctrl.c meson_usbphy.c mesong12_clkc.c mesongx_mmc.c
> mesongxbb_clkc.c
> src/sys/arch/arm/broadcom: bcm2835_emmc.c bcm2835_intr.c
> src/sys/arch/arm/fdt: gicv3_fdt.c pcihost_fdt.c
> src/sys/arch/arm/nvidia: tegra_ahcisata.c tegra_nouveau.c tegra_pcie.c
> tegra_pinmux.c tegra_soctherm.c tegra_xusb.c
> src/sys/arch/arm/nxp: if_enet_imx.c imx6_pcie.c imx6_spi.c
> imx8mq_usbphy.c imx_sdhc.c
> src/sys/arch/arm/rockchip: rk3328_iomux.c rk3399_iomux.c rk_gmac.c
> rk_i2s.c rk_pwm.c rk_tsadc.c rk_usb.c rk_v1crypto.c rk_vop.c
> src/sys/arch/arm/samsung: exynos_dwcmmc.c exynos_pinctrl.c
> exynos_platform.c exynos_usbdrdphy.c exynos_usbphy.c
> src/sys/arch/arm/sociox: if_ave.c
> src/sys/arch/arm/sunxi: sun4i_a10_ccu.c sun4i_dma.c sun6i_dma.c
> sun8i_crypto.c sunxi_can.c sunxi_codec.c sunxi_de2_ccu.c
> sunxi_dep.c sunxi_gpio.c sunxi_hdmi.c sunxi_hdmiphy.c sunxi_i2s.c
> sunxi_lcdc.c sunxi_lradc.c sunxi_mixer.c sunxi_mmc.c sunxi_musb.c
> sunxi_nmi.c sunxi_pwm.c sunxi_rsb.c sunxi_rtc.c sunxi_sid.c
> sunxi_sramc.c sunxi_tcon.c sunxi_thermal.c sunxi_ts.c sunxi_twi.c
> sunxi_usb3phy.c sunxi_usbphy.c sunxi_wdt.c
> src/sys/arch/arm/ti: ti_dpll_clock.c ti_gpio.c ti_iic.c ti_omapintc.c
> ti_omaptimer.c ti_sdhc.c
> src/sys/arch/macppc/dev: deq.c lmu.c psoc.c smusat.c
> src/sys/arch/mips/cavium/dev: octeon_cib.c octeon_intc.c
> src/sys/arch/sparc64/dev: pcf8591_envctrl.c
>
> Log Message:
> Since we're using designated initialisers for compat data, we should
> use a completely empty initializer for the sentinel.
>
> static const struct device_compatible_entry compat_data[] = {
> { .compat = "ecadc" },
> -
> - { 0 }
> + { }
> };
>
> static int
>
I have no problem with this change but I am curious why should we use "{
}"? It's a C GNU extension and C++ syntax.
signature.asc
Description: OpenPGP digital signature
Re: CVS commit: src/sys/dev/pci
> On Jan 24, 2021, at 10:20 PM, Martin Husemann wrote: > >> I kept getting a ?static after non-static declaration? error when building >> for aarch64. > > I guess that was with outdated arm/include/bus_funcs.h and > sys/bus_proto.h (or where was the previous declaration)? I did a “cvs update” just before, *shrug*. In any case, the problem was easily avoidable, and now it is avoided. -- thorpej
