[Openais] Re: Some event service code

[Steven Dake]

Got that set.

I wanted to place this hint in the c/h file itself

maybe its not possible.

Regards
-steve
On Thu, 2004-07-29 at 12:09, Sabharwal, Atul wrote:
> ON file load, do se ts=4 would set it in vi. You can store this in
> .vimrc
> And it will do it at file load automatically.
> 
> --
> Atul
> 
> -------------------------------------------------------------
> P.S:  All opinions are my personal opinion(s) & responsibility and do
> not represent the view of my employer ( Intel Corporation ).
>  
> 
> >-----Original Message-----
> >From: openais-bounces at lists.osdl.org 
> >[mailto:openais-bounces at lists.osdl.org] On Behalf Of Mark Haverkamp
> >Sent: Thursday, July 29, 2004 8:15 AM
> >To: Steven Dake
> >Cc: Openais List
> >Subject: [Openais] Re: Some event service code
> >
> >On Wed, 2004-07-28 at 16:13, Steven Dake wrote:
> >> Awesome.  Comments below...
> >> 
> >> On Wed, 2004-07-28 at 15:29, Mark Haverkamp wrote:
> >> > Here is a patch for the event service.  The library side 
> >is pretty far
> >> > along. The aisexec side has placeholders that just print 
> >something and
> >> > return status.  I just wanted to run this by you for a 
> >review before I
> >> > get too much farther into it.  I hope the coding style 
> >isn't too far
> >> > off.  I've tried to reuse as much of my existing code as I 
> >could.  It's
> >> > almost 2800 lines so rather than attach it, I put it here:
> >> > 
> >> > 
> >http://developer.osdl.org/markh/openais/openais_event_20040728.patch
> >> > 
> >> > Thanks,
> >> > Mark.
> >> 
> >> Mark
> >> 
> >> Wow I must say I am impressed.  Excellent work !!
> >> 
> >> Thanks for the dependencies..  That was driving me crazy 
> >when I edited
> >> the h files.
> >> 
> >> I do have a few comments of course :)
> >> 
> >> On the library:
> >> 
> >> I'd like the tabstops to look good at 4.  I think you are 
> >using tabstops
> >> of 8.  This is a minor thing.  I think there is a way to tell vi to
> >> force tabstops when it loads a file.  Do you know how this works?
> >
> >I've always used tabstops of 8.  I didn't notice that your code was set
> >up for tabs at 4 since your code seems to look OK at 8. I 
> >don't know how
> >to set the tabstops at file load.  I think that there were some kernel
> >files that had some emacs comments to do that, I'm not sure about vi.
> >
> >It looks like the only thing that looks funny is that the structure
> >declarations don't line up well at 4.    I'll run through the files and
> >line things up. 
> >
> >> 
> >> we could probably put clust_time_now in ais_util.h/.c
> >
> >Is that util.[ch] in the lib directory or new files?
> > 
> >> 
> >> In initialize, SaServiceConnect should free the queue allocated with
> >> saQueueInit if there is an error.
> >> 
> >> In channel open, you don't do a handle put if handlecreate fails
> >> 
> >> in channel close, the first handle instance isn't put if the second
> >> handle instance get fails.
> >
> >Thanks, I knew that I probably missed a few things like that here and
> >there.
> >
> >> 
> >> All functions not specifically exported should be static: patt_size,
> >> aispatt_to_evt_patt, filt_size, aisfilt_to_evt_filt
> >
> >Good idea.  I normally try to do this.
> >
> >> 
> >> I know some of the code in clm, amf, checkpointing may not 
> >follow these
> >> static guidelines, so if you see anything in any of the code, let me
> >> know so I can fix it up.
> >> 
> >> It might help parallel style a little bit if the header names are
> >> "header" vs ico_head, etc.  This definately isn't a big deal though.
> >
> >The thing that I like about the prefix names is that when I use cscope
> >to browse the source code, I can easily distinguish different structure
> >elements.
> >
> >> 
> >> You will have to change your executive handlers to take the struct
> >> in_addr parameter in the latest tree.
> >
> >OK.
> >
> >> 
> >> One big problem to be avoided is duplicating services in several
> >> places.  When I started openais some time ago, I had
> >> amfComponentRegister, and there were two versions.  One for 
> >the library
> >> which registered a component, and one for the executive.  
> >This creates a
> >> race condition.  If you send the message over the executive 
> >and handle
> >> the request in one place "in your TODO: Add open code here" the race
> >> conditions can be avoided  This can be applied all over the 
> >executive.
> >
> >Could you explain this again?  I'm not sure I understand what you are
> >saying here.
> >
> >> 
> >> If you can fix up the library comments I'll merge that as 
> >one patch, or
> >> if you want to wait for the entire thing we can.
> >
> >I'll update the code and send out another patch for you to merge,
> >probably this afternoon.  Then I'll start filling in the server side
> >code and send updates as things get working.  If that sounds OK.
> >
> >> 
> >> I'm looking forward to see the remainder of the executive service
> >> handler.
> >> 
> >
> >Thanks for the review.
> >Mark.
> >
> >
> >> Regards
> >> -steve
> >-- 
> >Mark Haverkamp <markh at osdl.org>
> >
> >