On 7/15/2010 4:03 AM, Alan Bateman wrote:
Daniel D. Daugherty wrote:
:
src/os/linux/vm/attachListener_linux.cpp
The addition of UNIX_PATH_MAX means that the size of the buffer
on line 61: static char _path[PATH_MAX+1]
is now or can be potentially out of sync with the size of the
buffer on line 170 char path[UNIX_PATH_MAX]
If UNIX_PATH_MAX happens to be greater than PATH_MAX, and the
path to the socket is actually longer than PATH_MAX, then we'll
store a short path in the buffer on line 61.
Is there some reason not to use the new UNIX_PATH_MAX in both
locations?
Thanks Dan, I missed that one although it shouldn't cause a problem
(as UNIX_PATH_MAX (108) will be less than PATH_MAX (usually 4096)).
Your choice on how to resolve this one.
src/os/solaris/vm/attachListener_solaris.cpp
The return values from snprintf() aren't checked here for potential
overflow conditions. It seems to me that the Solaris logic could
benefit from the same sanity checks that you make in the Linux
version.
I didn't change or add this as it didn't seem likely that we would be
started with java.io.tmpdir property set to a location that is close
to the maximum path length. I suspect other things (like
File.createTempFile) would also have problems if java.io.tmpdir were
set this way. It is easily fixed to check for truncation, if you think
it is important. The reason I added the check for truncation in the
Linux implementation is because it's a path to a socket file.
And it's a much shorter path. I would be happier if we had a
truncation check, but I'm okay if we don't.
I would keep the warnings debug only. I see you have a debug only
warning in the Solaris code, but no warning in the Linux code. Or
were you talking about a different warning?
I was wondering if I should additional warnings into this code, and if
so, if they should be in the product build. I know warning messages
can cause products when intermingled with application output.
Is there a TraceAttachOnDemand flag? If there is, then I would be
okay if the product bits output a message when that option is
enabled. Otherwise, I would just make it debug only.
Dan
