This is the mail archive of the java-patches@gcc.gnu.org mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: PlainSocketImpl changes to support socket timeouts (again)


>>> Tom Tromey <tromey@redhat.com> 31-Jul-01 5:21:44 AM >>>

formating comments: ok (some of them are style typos commited
elsewhere that I just copied).


    Nic> +  /** a cached copy of the in stream.
    Nic> +   */
    Nic> +  private InputStream in=null;

    Tom:
    Use `in = null' -- the spaces are required by the coding 
    standard.

Is there a working indent for java code? 

The Classpath people suggest use of indent with a patch one but no
one has been able to tell me which version of indent the patch applies
to.


    This line is fine -- it doesn't have to be wrapped 
    since there is no sensible (according to our style) place 
    to wrap it.

Are your style guides more than the GNU coding standard?


   Maybe a compiler bug?  That's what it sounds like.

Yeah. I don't have time to do any more tests so I wanted to leave a
comment in the code, more for myself really.

I can commit a working copy sans comment.


   Nic> +  // FIXME: loop if r != 1.

   I fixed this bug in natFileDescriptorPosix.cc.
   I think the fixes should be carried into this code.

Ok. I'll do that.


    Nic> +    PlainSocketImpl socket;
    Nic> +    
    Nic> +    SocketInputStream(PlainSocketImpl socket)

    A non-static inner class will have an implicit reference to its
    creating object.  So there is no need to have `socket' as a field
or
    as an argument to the constructor.

Yes. I like to do this for readability. I presume that the compiler
will remove (or homogenise) the dud reference.

But I'll change it if you want.


   I don't mean to pick so many nits.  

Yes you do /8->


   However, the coding style is important to us; I've regretted
letting
   in patches that don't conform.

However, without a conforming indent it's a real pain to reproduce.
It annoys me the Emacs doesn't automatically add spaces around = and
so forth.


   Thanks for doing this work.  It is something we've wanted 
   for a long time, but that nobody has been interested enough 
   to do.  It's an important contribution.

Unfortunately I'm now working full time with 2 hours travelling at
either end of the day, this considrably reduces the time I have to
work on stuff.

I probably won't get round to this particular job till the weekend.


Nic


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]