Re: A method with return type size_t returns negative value
On 28/11/2011 08:36, Jing Lv wrote: Hello Alan, I am fine with the fix on the webrev, suppose it would be applied later? I don't remember (as it's been a long time) exactly I found the issue, maybe some code scan or some strong compiler :) (Problem with my mail server so I can only see your mail on website archive) Okay, I think we should go with the changes that I have in the webrev: http://cr.openjdk.java.net/~alanb/7030624/webrev/ Could I get a reviewer for this? As a reminder, the issue that Jing has brought up a few times is that this code is using size_t which is unsigned. The changes in the webrev change the return type to jint. -Alan.
Re: A method with return type size_t returns negative value
On 11/29/11 06:12 PM, Lance Andersen - Oracle wrote: looks ok to me... +1 -Chris. On Nov 29, 2011, at 11:19 AM, Alan Bateman wrote: On 28/11/2011 08:36, Jing Lv wrote: Hello Alan, I am fine with the fix on the webrev, suppose it would be applied later? I don't remember (as it's been a long time) exactly I found the issue, maybe some code scan or some strong compiler :) (Problem with my mail server so I can only see your mail on website archive) Okay, I think we should go with the changes that I have in the webrev: http://cr.openjdk.java.net/~alanb/7030624/webrev/ Could I get a reviewer for this? As a reminder, the issue that Jing has brought up a few times is that this code is using size_t which is unsigned. The changes in the webrev change the return type to jint. -Alan. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: A method with return type size_t returns negative value
Hello,
I search the E:\workspace\openjdk\jdk\src\windows\native but find
nothing similar (please tell me if I miss something), so I suggest we
have a quick fix for now, like:
--- E:\workspace\openjdk\jdk\src\windows\native\java\io\io_util_md.h~
2011-11-15 15:53:21.0 +0800
+++ E:\workspace\openjdk\jdk\src\windows\native\java\io\io_util_md.h
2011-11-15 15:56:27.0 +0800
@@ -36,14 +36,14 @@
WCHAR* currentDir(int di);
int currentDirLength(const WCHAR* path, int pathlen);
void fileOpen(JNIEnv *env, jobject this, jstring path, jfieldID fid,
int flags);
int handleAvailable(jlong fd, jlong *pbytes);
JNIEXPORT int handleSync(jlong fd);
int handleSetLength(jlong fd, jlong length);
-JNIEXPORT size_t handleRead(jlong fd, void *buf, jint len);
-JNIEXPORT size_t handleWrite(jlong fd, const void *buf, jint len);
+JNIEXPORT SSIZE_T handleRead(jlong fd, void *buf, jint len);
+JNIEXPORT SSIZE_T handleWrite(jlong fd, const void *buf, jint len);
jint handleClose(JNIEnv *env, jobject this, jfieldID fid);
jlong handleLseek(jlong fd, jlong offset, jint whence);
/*
* Returns an opaque handle to file named by path. If an error occurs,
* returns -1 and an exception is pending.
--- E:\workspace\openjdk\jdk\src\windows\native\java\io\io_util_md.c~
2011-11-15 16:02:55.0 +0800
+++ E:\workspace\openjdk\jdk\src\windows\native\java\io\io_util_md.c
2011-11-15 16:03:08.0 +0800
@@ -484,13 +484,13 @@
}
if (SetEndOfFile(h) == FALSE) return -1;
return 0;
}
JNIEXPORT
-size_t
+SSIZE_T
handleRead(jlong fd, void *buf, jint len)
{
DWORD read = 0;
BOOL result = 0;
HANDLE h = (HANDLE)fd;
if (h == INVALID_HANDLE_VALUE) {
@@ -509,13 +509,13 @@
return -1;
}
return read;
}
JNIEXPORT
-size_t
+SSIZE_T
handleWrite(jlong fd, const void *buf, jint len)
{
BOOL result = 0;
DWORD written = 0;
HANDLE h = (HANDLE)fd;
if (h != INVALID_HANDLE_VALUE) {
I will put it on the bug system.
On 2011/11/9 16:57, Jing Lv wrote:
Hello Alan,
(Sorry that haven't check this for months) I check with the bug
7030624 but see no progress now. What's the current status? Shall we
go on with a simple patch to fix the problem?
On 2011/7/21 21:32, Alan Bateman wrote:
Jing LV wrote:
Ping, anyone notice this? :)
My plan is that we'd search all source to find all kinds of such
issues, and then refine them in a uniform solution. If jint is not
good enough, how about ssize_t?
ACK, saw your mails and will get back to you soon on this.
-Alan
Re: A method with return type size_t returns negative value
On 15/11/2011 08:09, Jing Lv wrote: Hello, I search the E:\workspace\openjdk\jdk\src\windows\native but find nothing similar (please tell me if I miss something), so I suggest we have a quick fix for now, like: I'm not around this week but I will get back to you next week on this (as I may have mentioned, I had hoped we would change all of this as part of other cleanup that includes removing dependencies on the JVM* functions). -Alan.
Re: A method with return type size_t returns negative value
Hello Alan, (Sorry that haven't check this for months) I check with the bug 7030624 but see no progress now. What's the current status? Shall we go on with a simple patch to fix the problem? On 2011/7/21 21:32, Alan Bateman wrote: Jing LV wrote: Ping, anyone notice this? :) My plan is that we'd search all source to find all kinds of such issues, and then refine them in a uniform solution. If jint is not good enough, how about ssize_t? ACK, saw your mails and will get back to you soon on this. -Alan
Re: A method with return type size_t returns negative value
Hello, It's been quite a long time and it seems jdk7 is doing well now. I know there is still several days before its release, anyway, Alan or someone else, would you please tell me if we can start work on jdk8, and restart the discussion on the bugs like this (I remember there are still some to be discussed)? Thanks! 于 2011-5-5 14:22, Jing LV 写道: 于 2011-4-26 16:43, Alan Bateman 写道: Jing LV wrote: Hello, Thanks for raising the defect. I see there is no patch now, so I create one (as the defect mentioned jint may be good, I use jint here). I have no Sun Online Account Id, would someone take a look? The function prototypes would also require to be updated and there are a couple of other areas that would require clean-up too. If it's okay with you, I think we should postpone this to jdk8. This code has been using size_t for many years and I don't think is actually causing a real problem. I agree it should be cleaned up but we are out of time in jdk7. Another point is that jdk8 will be our opportunity to remove the dependencies on the JVM_* functions and so this code will be changing anyway (I realize we're not using these in the Windows code but the functions need to match as they are used from shared code). -Alan. Hi Alan, I am OK with Java8. And I am wondering if a portlib or something may help. I'd think deeper and provide some proposal. -- Best Regards, Jimmy, Jing LV
Re: A method with return type size_t returns negative value
于 2011-4-26 16:43, Alan Bateman 写道: Jing LV wrote: Hello, Thanks for raising the defect. I see there is no patch now, so I create one (as the defect mentioned jint may be good, I use jint here). I have no Sun Online Account Id, would someone take a look? The function prototypes would also require to be updated and there are a couple of other areas that would require clean-up too. If it's okay with you, I think we should postpone this to jdk8. This code has been using size_t for many years and I don't think is actually causing a real problem. I agree it should be cleaned up but we are out of time in jdk7. Another point is that jdk8 will be our opportunity to remove the dependencies on the JVM_* functions and so this code will be changing anyway (I realize we're not using these in the Windows code but the functions need to match as they are used from shared code). -Alan. Hi Alan, I am OK with Java8. And I am wondering if a portlib or something may help. I'd think deeper and provide some proposal. -- Best Regards, Jimmy, Jing LV
Re: A method with return type size_t returns negative value
Hello,
Thanks for raising the defect. I see there is no patch now, so I
create one (as the defect mentioned jint may be good, I use jint here).
I have no Sun Online Account Id, would someone take a look?
diff --git a/src/windows/native/java/io/io_util_md.c
b/src/windows/native/java/io/io_util_md.c
index 345f955..50bc988 100644
--- a/src/windows/native/java/io/io_util_md.c
+++ b/src/windows/native/java/io/io_util_md.c
@@ -478,7 +478,7 @@ handleSetLength(jlong fd, jlong length) {
}
JNIEXPORT
-size_t
+jint
handleRead(jlong fd, void *buf, jint len)
{
DWORD read = 0;
@@ -502,7 +502,7 @@ handleRead(jlong fd, void *buf, jint len)
return read;
}
-static size_t writeInternal(jlong fd, const void *buf, jint len,
jboolean append)
+static jint writeInternal(jlong fd, const void *buf, jint len, jboolean
append)
{
BOOL result = 0;
DWORD written = 0;
@@ -527,16 +527,16 @@ static size_t writeInternal(jlong fd, const void
*buf, jint len, jboolean append
if ((h == INVALID_HANDLE_VALUE) || (result == 0)) {
return -1;
}
-return (size_t)written;
+return (jint)written;
}
JNIEXPORT
-size_t handleWrite(jlong fd, const void *buf, jint len) {
+jint handleWrite(jlong fd, const void *buf, jint len) {
return writeInternal(fd, buf, len, JNI_FALSE);
}
JNIEXPORT
-size_t handleAppend(jlong fd, const void *buf, jint len) {
+jint handleAppend(jlong fd, const void *buf, jint len) {
return writeInternal(fd, buf, len, JNI_TRUE);
}
于 2011-3-24 20:34, Alan Bateman 写道:
Jing LV wrote:
:
Thanks Alan. Would you please tell me the bug number?
The bug now tracking this is:
7030624: size_t usages in src/windows/native/java/io/io_util_md.c need
to be re-visited
-Alan.
--
Best Regards,
Jimmy, Jing LV
Re: A method with return type size_t returns negative value
Jing LV wrote: Hello, Thanks for raising the defect. I see there is no patch now, so I create one (as the defect mentioned jint may be good, I use jint here). I have no Sun Online Account Id, would someone take a look? The function prototypes would also require to be updated and there are a couple of other areas that would require clean-up too. If it's okay with you, I think we should postpone this to jdk8. This code has been using size_t for many years and I don't think is actually causing a real problem. I agree it should be cleaned up but we are out of time in jdk7. Another point is that jdk8 will be our opportunity to remove the dependencies on the JVM_* functions and so this code will be changing anyway (I realize we're not using these in the Windows code but the functions need to match as they are used from shared code). -Alan.
Re: A method with return type size_t returns negative value
On 26/04/11 09:43, Alan Bateman wrote: . Another point is that jdk8 will be our opportunity to remove the dependencies on the JVM_* functions That sounds interesting Alan - care to share more?
Re: A method with return type size_t returns negative value
Steve Poole wrote: On 26/04/11 09:43, Alan Bateman wrote: . Another point is that jdk8 will be our opportunity to remove the dependencies on the JVM_* functions That sounds interesting Alan - care to share more? As you know, it's really hard to remove things from the JDK. Medals are few. One thing that we've been trying to get rid of for many years is legacy interruptible I/O support, a troublesome mis-feature that dates back to jdk1.0. This is why older areas of the libraries (java.io and java.net specifically) call into the VM via JVM_* functions to do I/O. Back in jdk6 we added a VM option to allow the mechanism be disabled. Early in jdk7 the default option was changed so that it is now disabled by default. Once jdk8 opens then we should be able to remove the mechanism and clean-up the library code so that it no longer calls into the VM. My point to Jing LV was that this would seem a more appropriate time to do the clean-up he is suggesting. -Alan.
Re: A method with return type size_t returns negative value
Steve Poole wrote: Ok Alan - thanks for the background. I must of missed the VM option along the way - what's it called? It's HotSpot specific and is named UseVMInterruptibleIO. -Alan
Re: A method with return type size_t returns negative value
于 2011-3-23 18:23, Alan Bateman 写道: Jing LV wrote: Hello, I found a problem in windows/native/java/io/io_util_md.c, method handleRead(). The return type of the method is size_t, which is defined as unsigned int. However we see the method would return negative values while it meets some error. This may be a problem as the API comsumer and later developer may have misunderstand it. I suggest modify it to ssize_t (or SSIZE_T as it is on windows). Any comments? I suspect this just haven't been noticed before. I checked the pre-OpenJDK history and these handle* functions have been returning size_t since the code was changed from using C runtime functions to win32 functions (in 2003). I don't think we have any resulting bugs but it should be cleaned up so I'll create a bug to track it. I see there are a few other places that have the same issue. -Alan. Thanks Alan. Would you please tell me the bug number? -- Best Regards, Jimmy, Jing LV
Re: A method with return type size_t returns negative value
Jing LV wrote: : Thanks Alan. Would you please tell me the bug number? The bug now tracking this is: 7030624: size_t usages in src/windows/native/java/io/io_util_md.c need to be re-visited -Alan.
A method with return type size_t returns negative value
Hello, I found a problem in windows/native/java/io/io_util_md.c, method handleRead(). The return type of the method is size_t, which is defined as unsigned int. However we see the method would return negative values while it meets some error. This may be a problem as the API comsumer and later developer may have misunderstand it. I suggest modify it to ssize_t (or SSIZE_T as it is on windows). Any comments? -- Best Regards, Jimmy, Jing LV
Re: A method with return type size_t returns negative value
Jing LV wrote: Hello, I found a problem in windows/native/java/io/io_util_md.c, method handleRead(). The return type of the method is size_t, which is defined as unsigned int. However we see the method would return negative values while it meets some error. This may be a problem as the API comsumer and later developer may have misunderstand it. I suggest modify it to ssize_t (or SSIZE_T as it is on windows). Any comments? I suspect this just haven't been noticed before. I checked the pre-OpenJDK history and these handle* functions have been returning size_t since the code was changed from using C runtime functions to win32 functions (in 2003). I don't think we have any resulting bugs but it should be cleaned up so I'll create a bug to track it. I see there are a few other places that have the same issue. -Alan.
