Re: [PATCH] for Review: SelectorImpl.wakeup() + Synchronization

[Mohan Embar <gnustuff at thisiscool dot com>]

Hi Michael,

Thanks for looking at this.

>Method and field documentation should look like this:
>
>  /**
>   * Some text ....
>   */
>  private boolean unhandledWakeup;

(What was I thinking? I've probably been in the .cc files for too long.)

>> +  private boolean unhandledWakeup = false;
>
>false is the default value of boolean and not explicitely needed.

Is this the official libgcj stance? I understand default values in Java,
but as a matter of style, I initialize all fields either explicitly
or in the constructor. This being said, I'm searching the web
for some document to back me up and I can't find anything
after a quick search....

>> +      throw new ClosedSelectorException ();
>
>Dont add "space" between method/constructor name and '('. There are
>serveral instances of this misformatting. I know I said different in the
>past but tromey enlightened me.

Are we doing this differently than C?:

http://www.gnu.org/prep/standards_23.html#SEC23

If so, why? (Not that I'm complaining about not having this space, I'd
gladly remove these from the C/C++ code too if Father GNU didn't make
me put them there.)

>> -  /**
>> -   * Marks the beginning of an I/O operation that might block indefinitely.
>> -   */
>> -  protected final void begin()
>> -  {
>> -  }
>> -
>> -  /**
>> -   * Marks the end of an I/O operation that might block indefinitely.
>> -   */
>> -  protected final void end()
>> -  {
>> -  }
>> -    
>
>Why do you remove methods required by the official API ?

Oops. I should have realized that this was a "real" class. I'll put this
back in, but based on my implementation, do you think these really
need to be overridden in SelectorImpl? I wouldn't know what I'd put
in them.

-- Mohan
http://www.thisiscool.com/
http://www.animalsong.org/