RFR(S): 8009397 test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket
David Holmes
david.holmes at oracle.com
Tue Mar 19 01:09:53 UTC 2013
Hi Serguei,
On 19/03/2013 2:47 AM, serguei.spitsyn at oracle.com wrote:
> I already reviewed this, and it is a good fix.
> Please, see my email attached.
You are not an OpenJDK Reviewer [1] . Staffan needs an official Review
from a Reviewer before it can be pushed. That doesn't mean your review
is not wanted or appreciated - it mostly certain is - but an official
Review is still required.
Cheers,
David
[1] http://openjdk.java.net/bylaws#reviewer
> Thanks,
> Serguei
>
> On 3/18/13 7:13 AM, Staffan Larsen wrote:
>> Can I get an official Review of this change?
>>
>> Thanks,
>> /Staffan
>>
>> On 7 mar 2013, at 12:54, Staffan Larsen <staffan.larsen at oracle.com
>> <mailto:staffan.larsen at oracle.com>> wrote:
>>
>>> Here is a webrev for fixing this problem. I actually removed all of
>>> our own tokenization code in dll_build_name() and replaced it with
>>> calls to strtok_r (strtok_s on windows) instead. I think this should
>>> be more robust, at the cost of an extra memory allocation. I've also
>>> added the const qualifier to some of the parameters.
>>>
>>> http://cr.openjdk.java.net/~sla/8009558/webrev.02/
>>> <http://cr.openjdk.java.net/%7Esla/8009558/webrev.02/>
>>>
>>> All of the jdi and hprof tests passes with this change.
>>>
>>> Thanks,
>>> /Staffan
>>>
>>> On 6 mar 2013, at 20:48, serguei.spitsyn at oracle.com
>>> <mailto:serguei.spitsyn at oracle.com> wrote:
>>>
>>>> Staffan,
>>>>
>>>> Thank you for the confirmation and taking care about this issue!
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>> On 3/6/13 11:36 AM, Staffan Larsen wrote:
>>>>> Nice catch, Serguei!
>>>>>
>>>>> Unfortunately I have already pushed my change, but I filed
>>>>> "JDK-8009558: linked_md.c::dll_build_name can get stuck in an
>>>>> infinite loop" to track this new problem and I am working on a fix.
>>>>>
>>>>> Thanks,
>>>>> /Staffan
>>>>>
>>>>>
>>>>> On 5 mar 2013, at 20:26, serguei.spitsyn at oracle.com
>>>>> <mailto:serguei.spitsyn at oracle.com> wrote:
>>>>>
>>>>>> Hi Staffan,
>>>>>>
>>>>>> Thank you for this discovery!
>>>>>> It looks good, but I have a couple of comments.
>>>>>>
>>>>>> It seems, there is one more problem in this code:
>>>>>> 68 /* check for NULL path */
>>>>>> 69 if (p == pathname) {
>>>>>> 70 continue;<== Endless loop if we hit this line
>>>>>> 71 }
>>>>>>
>>>>>> Do we need to do 'pathname++' before continuing at the line #70?
>>>>>> It is going to be endless loop in cases there is a PATH_SEPARATOR
>>>>>> at the beginning of paths or two PATH_SEPARATOR's in a row.
>>>>>> These would be incorrect path lists but the code above is
>>>>>> targeting exactly such cases.
>>>>>>
>>>>>> Also, the argument name "pathname" in the original code is confusing.
>>>>>> Should we rename it to "pathnames"?
>>>>>>
>>>>>> Thanks,
>>>>>> Serguei
>>>>>>
>>>>>>
>>>>>> On 3/4/13 10:56 AM, Staffan Larsen wrote:
>>>>>>> I accidentally stepped on this bug the other day. There is a
>>>>>>> problem in linker_md.c : dll_build_name() where an internal
>>>>>>> pointer can be moved to point outside the string. The code looks
>>>>>>> like:
>>>>>>>
>>>>>>> 57 static void dll_build_name(char* buffer, size_t buflen,
>>>>>>> 58 const char* pname, const char*
>>>>>>> fname) {
>>>>>>> 59 // Based on os_solaris.cpp
>>>>>>> 60
>>>>>>> 61 char *path_sep = PATH_SEPARATOR;
>>>>>>> 62 char *pathname = (char *)pname;
>>>>>>> 63 while (strlen(pathname) > 0) {
>>>>>>> 64 char *p = strchr(pathname, *path_sep);
>>>>>>> 65 if (p == NULL) {
>>>>>>> 66 p = pathname + strlen(pathname);
>>>>>>> 67 }
>>>>>>> 68 /* check for NULL path */
>>>>>>> 69 if (p == pathname) {
>>>>>>> 70 continue;
>>>>>>> 71 }
>>>>>>> 72 (void)snprintf(buffer, buflen, "%.*s/lib%s."
>>>>>>> LIB_SUFFIX, (p - pathname),
>>>>>>> 73 pathname, fname);
>>>>>>> 74
>>>>>>> 75 if (access(buffer, F_OK) == 0) {
>>>>>>> 76 break;
>>>>>>> 77 }
>>>>>>> 78 pathname = p + 1;
>>>>>>> 79 *buffer = '\0';
>>>>>>> 80 }
>>>>>>>
>>>>>>> If the supplied pname is a buffer with a simple path without any
>>>>>>> path separators in it, p will be set to the terminating nul on
>>>>>>> line 66. Then on line 78 it will be moved outside the buffer.
>>>>>>> Fixing this also necessitates fixes to the callers to check for
>>>>>>> an empty return string (in buffer).
>>>>>>>
>>>>>>> The same code show up in both the solaris code and the windows
>>>>>>> code as well as hprof.
>>>>>>>
>>>>>>> bug:http://bugs.sun.com/view_bug.do?bug_id=8009397
>>>>>>> webrev:http://cr.openjdk.java.net/~sla/8009397/webrev.00/
>>>>>>>
>>>>>>> Thanks,
>>>>>>> /Staffan
>>>>>>
>>>>>
>>>>
>>>
>>
>
More information about the core-libs-dev
mailing list