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]
Other format: [Raw text]

Re: Patch for Review: JvGetStringUTF and JvTempCString


Hi Bryce,

Thanks for looking at this.

>Sorry if I missed earlier discussion on this. What is the rationale for 
>adding these?

Check out java/io/natFilePosix.cc. Incessantly repeated lines like this:

  char *buf = (char *) __builtin_alloca (JvGetStringUTFLength (path) + 1);
  jsize total = JvGetStringUTFRegion (path, 0, path->length(), buf);
  buf[total] = '\0';

...which were also present in the Win32 implementation, were driving
me nuts.

>I don't like JvGetStringUTF because it is unsafe - it 
>knows nothing about the size of the buffer it is writing into. I think 
>a variant which accepts the buffer size as an argument, and makes sure 
>not to overrun it, would be better.

This was gnawing at me too. I rationalized it by reasoning that they
could call JvGetStringUTFLength() if they were really paranoid, or else
just use JvTempCString (but the slight gnawing feeling never really went away).
I'm okay with this suggestion.

>As for JvTempCString, do you have an example of when this is needed? 
>What is wrong with, in client code, simply using JvGetStringUTF into a 
>stack allocated buffer?

I wanted this for all of the nat*.cc files where those three lines were
incessantly repeated. Your suggestion of a stack-allocated buffer doesn't
solve the problem either, because I then need to:

- allocated a fixed-size buffer, then see whether the allocation was
  successful, etc. etc. etc., or
- use __builtin_alloca() and incessantly repeat the above three lines if I don't
  have JvGetStringUTFChars(), or
- incessantly repeat the two (IMO clunky) lines if I do have JvGetStringUTFChars().

It's also worth noting that not everyone knows about __builtin_alloca() and its
proper usage. Do a search on _Jv_Free in nat*.cc and see what you get. Using
this class would eliminate this confusion, at least for the problem I'm trying to address
with this patch.

>gcj/cni.h is the place to put things which are to be part of the public 
>CNI interface.

I trust that Tom and you will work this one out.

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






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