Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 3 Aug 2017 05:51:42 -0700
From:      "Ngie Cooper (yaneurabeya)" <yaneurabeya@gmail.com>
To:        Bruce Evans <brde@optusnet.com.au>
Cc:        Ngie Cooper <ngie@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r321969 - in head/sys/boot: arm/at91/libat91 arm/ixp425/boot2 i386/boot2
Message-ID:  <D6B389AC-E57A-4E90-9477-0B4871DF8773@gmail.com>
In-Reply-To: <20170803173421.C2203@besplex.bde.org>
References:  <201708030527.v735R5dg041043@repo.freebsd.org> <20170803173421.C2203@besplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--Apple-Mail=_00D0BBC5-84C1-4464-844F-7D7F64E274A1
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8


> On Aug 3, 2017, at 00:53, Bruce Evans <brde@optusnet.com.au> wrote:
>=20
> On Thu, 3 Aug 2017, Ngie Cooper wrote:
>=20
>> Log:
>> Fix the return types for printf and putchar to match their libc and
>> POSIX equivalents
>>=20
>> Both printf and putchar return int, not void.
>>=20
>> This will allow code that leverages the libcalls and checks/rely on =
the
>> return type to interchangeably between loader code and non-loader
>> code.
>>=20
>> MFC after:	1 month
>>=20
>> Modified:
>> head/sys/boot/arm/at91/libat91/lib.h
>> head/sys/boot/arm/at91/libat91/printf.c
>> head/sys/boot/arm/at91/libat91/putchar.c
>> head/sys/boot/arm/ixp425/boot2/ixp425_board.c
>> head/sys/boot/arm/ixp425/boot2/lib.h
>> head/sys/boot/i386/boot2/boot2.c
>=20
> This is wrong for at least i386/boot2.  It isn't part of the loader, =
and
> saves space by not returning unused values.
>=20
>> Modified: head/sys/boot/i386/boot2/boot2.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=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D
>> --- head/sys/boot/i386/boot2/boot2.c	Thu Aug  3 03:45:48 2017	=
(r321968)
>> +++ head/sys/boot/i386/boot2/boot2.c	Thu Aug  3 05:27:05 2017	=
(r321969)
>> @@ -114,8 +114,8 @@ void exit(int);
>> static void load(void);
>> static int parse(void);
>> static int dskread(void *, unsigned, unsigned);
>> -static void printf(const char *,...);
>> -static void putchar(int);
>> +static int printf(const char *,...);
>> +static int putchar(int);
>=20
> These are freestanding static functions, so they have nothing to do
> with library functions except their name is a hint that they are
> similar.
>=20
> Since they are static, -funit-at-a-time might allow the unused return =
values
> to be optimized away.  Then returning unused values would be just an
> obfuscation.
>=20
> This file still has a static memcpy() which is quite different from =
the
> libc version.  It doesn't return an unused value, and its arg types =
are
> all different (no newfangled size_t or newerfangled restrict).
>=20
> Freestanding versions (static and otherwise) cause problems with =
builtins.
> -ffreestanding turns off all builtins.  The static memcpy used to be
> ifdefed so as to use __builtin_memcpy instead of the static one if the
> compiler is gcc.  That apparently broke with gcc-4.2, since the =
builtin
> will call libc memcpy() in some cases, although memcpy() is =
unavailable
> in the freestanding case.

I get the point about them being freestanding functions, but if the =
functions aren=E2=80=99t meant to be compatible they should be named =
differently. Part of the issue some code that bridged loader and =
non-loader users (bootdevtest and zfsboottest for example) relied on =
feature parity (in part because the ZFS code required it and because of =
how things are compiled/linked together). If compilers get pedantic =
enough, then they=E2=80=99ll flag these issues as errors and we=E2=80=99ll=
 have to address these compatibilities at that point.

My intent was ok (I think), but the implementation I did is wrong, so =
I=E2=80=99ll revert the change completely and leave it for someone else =
to deal with the incompatibilities (I=E2=80=99ll just integrate =
bootdevtest and zfsboottest into buildworld under sys/boot so they =
won=E2=80=99t remain broken for weeks on end, like they were recently).

Thanks,
-Ngie

--Apple-Mail=_00D0BBC5-84C1-4464-844F-7D7F64E274A1
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename=signature.asc
Content-Type: application/pgp-signature;
	name=signature.asc
Content-Description: Message signed with OpenPGP using GPGMail

-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - https://gpgtools.org

iQIcBAEBCgAGBQJZgxxeAAoJEPWDqSZpMIYV9cwQALEbGxM4T5iX8c8dSM8SnRXR
ZIU7so3LCYyB90eUAejhAmf9DD0VKkPq/Jt4btMpht8Y5zqZuskvcweuAxTZlZ+y
EDHVVrvz0afYO9iwjV3UVjVCE7Kh7aZ0PfH44Nu1SqseTxqd5mueG6Gi/oAXv0nu
+n35vZnbNcyl1ZpPrxp+UQ01TcyOJkjUHwW+Bzb4EXtE+7DRzIkO2popNWvJf12a
ht6iapBUWuTLj/zfPb2cSa2R+bWUeXBFVbGmP4ugIws/Nv2cgGbLpZw/EoAviYvq
vh/7r/pzQDQPqg/Zz0QOlprZ1TtwsnJ+XaLPHmjXYanFplCZ6NRtIp2AWnRX++AT
/7IQ0oaCUtos2oakqHgH9Xiw7vESpfSJxR8yZsYtYSH0xSG9gbR1gBBWCd0B1x6F
2Dvr5zzc2pOs9mDsPjiRsyU5lecKH1ULn81jIK/FSxmI34o0O/KjarYu4jDtabqE
NkxfvQBYAl3XALM1iSfNuo640/gPJoHGI5lpuLhz4Pl/a6pMTNXUBk+dE1EX9lOK
LkjtzqjAAvVly2tl4dLJWLPUeAoyjJNbOo2NbHzcp1zXwxqXEGFGwscY6ZIukCWy
6BGusj35HqWEQdhhgeRC2zXuLiQPnb/hkBSaVWTxisCL06rOgmnbBokkwgzVOXtp
WdQ5xacFWGHgp0l1iGGq
=TrgD
-----END PGP SIGNATURE-----

--Apple-Mail=_00D0BBC5-84C1-4464-844F-7D7F64E274A1--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?D6B389AC-E57A-4E90-9477-0B4871DF8773>