Please review 6938627: Make temporary directory use property java.io.tmpdir when specified

[Coleen Phillimore - Sun Microsystems]

Thanks David.
The ostream.cpp used the result of get_temp_directory() and didn't 
protect against string buffer overflow (which is unlikely given the 
buffer allocated was pretty large), but since I touched related code I 
had to fix this too.  I completely agree we need a bug to search for and 
fix the remaining sprintfs and strcpy's that can overflow buffer sizes.  
Apparently there are 176.

philli% ygrep sprintf | wc -l
     176

Thanks!
Coleen

On 03/30/10 19:02, David Holmes wrote:
> Hi Coleen,
>
> This looks okay.
>
> The changes in ostream.cpp seem incidental to this CR though.
>
> Good catch on the buffer overflow issue, but more generally we need a 
> CR to eliminate sprintf from the VM sources.
>
> David
>
> Coleen Phillimore - Sun Microsystems said the following on 03/31/10 
> 07:45:
>>
>> I've revised the /tmp directory code to protect against buffer 
>> overflows.  I've also tested it on linux too.  Please rereview this 
>> code.
>>
>> Summary: Get java.io.tmpdir property in os::get_temp_directory() and 
>> call this instead of harcoding "/tmp". Don't assume trailing 
>> file_separator either.
>>
>> local webrev at http://jruntime.east/~coleenp/webrev/6938627
>> bug link at http://monaco.sfbay.sun.com/detail.jsf?cr=6938627
>>
>> Thanks,
>> Coleen
>>
>> On 03/26/10 20:07, David Schlosnagle wrote:
>>> Coleen,
>>>
>>> There seems to be a couple issues on line 469 of 
>>> src/os/linux/vm/attachListener_linux.cpp:
>>> - It seems like there should be a slash in the format string to 
>>> match the Solaris version.
>>> - The call to os::get_temp_directory() appears to be missing the 'y' 
>>> at the end.
>>>
>>> src/os/linux/vm/attachListener_linux.cpp:
>>> 469     sprintf(fn, "%s.attach_pid%d", os::get_temp_director(),
>>>                        ^                                   ^^
>>> 470                                    os::current_process_id());
>>>
>>> src/os/solaris/vm/attachListener_solaris.cpp:
>>> 600     sprintf(fn, "%s/.attach_pid%d", os::get_temp_directory(),
>>>                        ^                                     ^^
>>> 601                                     os::current_process_id());
>>>
>>> Thanks,
>>> Dave
>>>
>>>
>>> On Mar 26, 2010, at 6:11 PM, Coleen Phillimore wrote:
>>>
>>>  
>>>> Summary: Get java.io.tmpdir property in os::get_temp_directory() 
>>>> and call this instead of harcoding "/tmp".  Don't assume trailing 
>>>> file_separator either.
>>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/6938627/
>>>> bug link at http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6938627
>>>>
>>>> Thanks,
>>>> Coleen
>>>>     
>>>
>>>