[Openais] Re: Some event service code
Sabharwal, Atul
atul.sabharwal at intel.com
Thu Jul 29 12:09:36 PDT 2004
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>
>
>
More information about the Openais
mailing list