Re: CVS commit: src/usr.sbin/rtsold

2014-03-18 Thread Christos Zoulas
On Mar 18,  9:08pm, da...@l8s.co.uk (David Laight) wrote:
-- Subject: Re: CVS commit: src/usr.sbin/rtsold

| Only if 'long' is smaller than 100.

In this particular case with these constants.

| i386 fails on the line:
| long interval = arc4random() /* uint32_t */ % 100;
| If you use 'unsigned long' then amd64 fails a line later doing:
| xxx.tv_usec = interval / 100;
| The compiler should be able to tell that neither of those lines
| is going to cause any problems.

I could have used uint32_t interval and then there would be no complaints.

| There is no reason to force 32bit systems to do unnecessary 64bit maths.

It is not like this case is performance critical.

christos


Re: CVS commit: src/usr.sbin/rtsold

2014-03-18 Thread David Laight
On Tue, Mar 18, 2014 at 08:21:19PM +, Christos Zoulas wrote:
> In article <20140318201420.go20...@snowdrop.l8s.co.uk>,
> David Laight   wrote:
> >On Tue, Mar 18, 2014 at 03:30:09PM -0400, Christos Zoulas wrote:
> >> Module Name:   src
> >> Committed By:  christos
> >> Date:  Tue Mar 18 19:30:09 UTC 2014
> >> 
> >> Modified Files:
> >>src/usr.sbin/rtsold: rtsold.c
> >> 
> >> Log Message:
> >> use time_t for time
> >
> >It isn't a time_t type time.
> 
> It is assigned to a time_t. long might not be wide enough; the value is
> unsigned and it is assigned to a signed type.

Only if 'long' is smaller than 100.
i386 fails on the line:
long interval = arc4random() /* uint32_t */ % 100;
If you use 'unsigned long' then amd64 fails a line later doing:
xxx.tv_usec = interval / 100;
The compiler should be able to tell that neither of those lines
is going to cause any problems.

There is no reason to force 32bit systems to do unnecessary 64bit maths.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/usr.sbin/rtsold

2014-03-18 Thread Christos Zoulas
In article <20140318201420.go20...@snowdrop.l8s.co.uk>,
David Laight   wrote:
>On Tue, Mar 18, 2014 at 03:30:09PM -0400, Christos Zoulas wrote:
>> Module Name: src
>> Committed By:christos
>> Date:Tue Mar 18 19:30:09 UTC 2014
>> 
>> Modified Files:
>>  src/usr.sbin/rtsold: rtsold.c
>> 
>> Log Message:
>> use time_t for time
>
>It isn't a time_t type time.

It is assigned to a time_t. long might not be wide enough; the value is
unsigned and it is assigned to a signed type.

christos



Re: CVS commit: src/usr.sbin/rtsold

2014-03-18 Thread David Laight
On Tue, Mar 18, 2014 at 03:30:09PM -0400, Christos Zoulas wrote:
> Module Name:  src
> Committed By: christos
> Date: Tue Mar 18 19:30:09 UTC 2014
> 
> Modified Files:
>   src/usr.sbin/rtsold: rtsold.c
> 
> Log Message:
> use time_t for time

It isn't a time_t type time.
The value is actually an interval in microseconds and bounded to less than
a second.

The code is doing:
long x = arc4random() % (1 * 100);
That really shouldn't generate a compiler warning.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/powerpc/powerpc

2014-03-18 Thread Christos Zoulas
In article <20140318143431.920a...@cvs.netbsd.org>,
Michael Lorenz  wrote:
>-=-=-=-=-=-
>
>+#ifdef PPC_OEA601
>+static struct timecounter powerpc_601_timecounter = {
>+  get_601_timecount,  /* get_timecount */
>+  0,  /* no poll_pps */
>+  0x7fff, /* counter_mask */
>+  0,  /* frequency */
>+  "rtc",  /* name */
>+  100,/* quality */
>+  NULL,   /* tc_priv */
>+  NULL/* tc_next */
>+};
>+#endif
>+
> static struct timecounter powerpc_timecounter = {
>   get_powerpc_timecount,  /* get_timecount */
>   0,  /* no poll_pps */
>   0x7fff, /* counter_mask */
>   0,  /* frequency */
>-#if PPC_OEA601
>-  "rtc",  /* name */
>-#else
>   "mftb", /* name */
>-#endif
>   100,/* quality */
>   NULL,   /* tc_priv */
>   NULL/* tc_next */

Can we use c99 field initializers here instead of comments?!?

christos