Re: [PATCH] linux-2.5.40_timer-changes_A3 (3/3 - integration)

[john stultz]

On Thu, 2002-10-03 at 00:28, Greg KH wrote:
> On Thu, Oct 03, 2002 at 12:13:54AM -0700, john stultz wrote:
> > On Wed, 2002-10-02 at 23:59, Greg KH wrote:
> > > > +/* fwd declarations */
> > > 
> > > These don't have to be forward declarations, do they?
> > > And can they be static?
> > 
> > Ummm. I could just be wrong, but since I'm setting structure elements to
> > equal the functions before they are declared, I need the fwds (unless,
> > of course I put the "struct timer_opts timer_pit" section below all the
> > functions, which is doable).
> 
> That's a bit nicer, that way you don't have to declare it twice, but
> it's not a big deal either way (no style rule here :)

True. I was just trying to keep the new structures at the top of the
code so they'd get the most review. I'll change it in my tree and either
update or resend depending on if this goes in. 
 

> > Also, since external functions are going to be calling these functions
> > via the structure's function pointers, I believe they can't be static.
> > Although, maybe they can, as long as the timer_pit value isn't static.
> > I'm not that much of a C guru, so I'm really sure.
> 
> No, they can be static, and they should, to keep the namespace a bit
> cleaner.  The pointer itself isn't static, and all references to the
> function goes through it, so the functions do not need to be global.

Sounds good. Thanks for clearing that up. 
 
> > > Shouldn't these 3 lines be above the "/* fwd declarations */" line?
> > 
> > They could be, but I'm not sure about the necessity. Is this a coding
> > style sorta' thing, or a C properness sort of thing? Either way is fine,
> > I just don't follow the logic. 
> 
> Just a "keep all #includes at the top of the file" type of thing, unless
> it's absolutely necessary.

Ok, will do.

Thanks again for the feedback.
-john