Re: [RFC PATCH 11/13] time sync: generic infrastructure to map between time stamps generated by a clock source and system time

[Patrick Ohly <patrick.ohly@intel.com>]

On Tue, 2008-11-11 at 16:18 +0000, Andi Kleen wrote:
> > +
> > +int clocksync_offset(struct clocksync *sync,
> > +		s64 *offset,
> > +		u64 *hwtstamp)
> > +{
> > +	u64 starthw = 0, endhw = 0;
> > +	struct {
> > +		s64 offset;
> > +		s64 duration_sys;
> > +	} samples[100], 
> 
> That should be separately allocated to avoid potential stack overflow.

Good catch. "make checkstack" also complains about it, but I didn't get
around to fixing it yet.

I'd prefer to allocate a very small array on the stack (10 entries = 160
bytes) and only fall back to dynamic allocation if the user of clocksync
wants more samples.

> Also as a style nit there are normally no {} around single line
> statements.

This is the part of the CodingStyle that I had most trouble adapting to
because a) I wrote a lot of code where the required style explicitly
asked for {} and b) I can think of several reasons for adding them
always and only one for not adding them.

Anyway, I'll try to keep this in mind, but would prefer to not reformat
the patches unless I have to touch them for other reasons.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.