From mboxrd@z Thu Jan 1 00:00:00 1970 From: Patrick Ohly Subject: Re: [RFC PATCH 11/13] time sync: generic infrastructure to map between time stamps generated by a clock source and system time Date: Wed, 12 Nov 2008 09:01:38 +0100 Message-ID: <1226476898.31699.37.camel@ecld0pohly> References: <1226414697.17450.852.camel@ecld0pohly> <1226415447.31699.10.camel@ecld0pohly> <4919B03E.1050504@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: "netdev@vger.kernel.org" , Octavian Purdila , Stephen Hemminger , Ingo Oeser , "Ronciak, John" , Eric Dumazet , Oliver Hartkopp To: Andi Kleen Return-path: Received: from mga11.intel.com ([192.55.52.93]:25890 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751308AbYKLIBz (ORCPT ); Wed, 12 Nov 2008 03:01:55 -0500 In-Reply-To: <4919B03E.1050504@linux.intel.com> Sender: netdev-owner@vger.kernel.org List-ID: 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.