Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 5 Mar 2010 04:26:10 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Garrett Cooper <yanefbsd@gmail.com>
Cc:        "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, Max Laier <max@love2party.net>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, Warner Losh <imp@freebsd.org>
Subject:   Re: svn commit: r204672 - head/sbin/newfs
Message-ID:  <20100305035406.I9077@delplex.bde.org>
In-Reply-To: <38B3C45B-B617-413F-868C-A25E3651CA59@gmail.com>
References:  <F80AA765-5CD7-4E69-8EF9-5A3E44853B35@gmail.com> <201003040036.25583.max@love2party.net> <38B3C45B-B617-413F-868C-A25E3651CA59@gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 3 Mar 2010, Garrett Cooper wrote:

> On Mar 3, 2010, at 3:36 PM, Max Laier <max@love2party.net> wrote:
>
>> On Thursday 04 March 2010 00:25:53 Garrett Cooper wrote:
>>> On Mar 3, 2010, at 1:53 PM, Warner Losh <imp@FreeBSD.org> wrote:
>>>> Author: imp
>>>> Date: Wed Mar  3 21:53:25 2010
>>>> New Revision: 204672
>>>> URL: http://svn.freebsd.org/changeset/base/204672
>>>> 
>>>> Log:
>>>> Cast these to intmax_t before printing to fix build bustage.  Better
>>>> solutions welcome.

Better solutions sent in private mail (basically, don't use int64_t to
hold small integers just because the expand_number() API is bad).

>>> Use PRId64 from inttypes.h ?
>> 
>> you just forgot to type ";)" there, didn't you?

:-).

> Eh, not really my intent TBH (although I guess my answer was pretty cheeky 
> looking back).
>
>> How are the PRI* macros
>> better than an intmax_t cast?

They reduce runtime bloat at a cost of source code size and ugliness bloat.

They should never be used, except perhaps on systems with 16K RAM.
Unfortunately, the corresponding macros for the scanf() family are
sometimes useful, since more than a simple cast would be needed instead.
Fortunately the scanf() family must never be used to scan integer
variables from user-supplied strings, since it gives undefined behaviour
on overflow, so scanf() is rarely useable.

> In my opinion, the intmax_t cast and %j is 
>> the
>> best, realistic solution for this.  I have partitioned in the past that we
>> change int64 types to "long long" on platforms with 64bit "long".  That way
>> you can print int64 types with "%ll" on all platforms.  But casting to 
>> intmax
>> is just as good and the compiler should be able to figure out what goes on 
>> and
>> make it a no-op.

You forgot to type ";)" in the past, didn't you?

Depending on that would add another layer of bogusness here.  The types
were changed from int to int64_t to match the broken-as-designed API
of expand_number() (it returns an int64_t indirectly, and it is convenient
though sloppy to point it to the final variable, so as not to add range
checking (though the necessary range check was nonexistent before).  This
API is broken since intmax_t is much more useful and easy to use than
int64_t, but the API is limited to int64_t.  It is also missing support
for unsigned values.  Anyway, with a non-broken API, but keeping the sloppy
type changes, all the types would have changed to intmax_t and there would
have been no problems with the printf formats provided they were changed to
match (no casts needed for intmax_t).

> Course it helps to read more dilligently too. It's fine today I suppose, but 
> pardoning my ignorance, are there any archs where in practice today, intmax_t 
> isn't synonymous with int64_t, like maybe some ARM variants?

Not that I know of.  But there are likely to be arches in practice tomorrow
(I guess 20 years for normal practice) where intmax_t is larger than int64_t.
Who knows what size long long will have then (it should have size -6 feet
but will take longer than 20 years to get there).  Why write broken code
that will have to be changed every 10-20 years when normal type sizes change?

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100305035406.I9077>