Re: [cp-patches] RFC: Object de-/serialization improvement
Hi there, Jeroen Frijters schrieb: Roman Kennke wrote: What about || (l == VMClassLoader.getSystemClassLoader()) instead ? You shouldn't call VMClassLoader.getSystemClassLoader() (it creates a new instance of the system class loader), but apart from that comparing against the system class loader would work, but I'd much rather see a solution that allows caching for other classes as well. So what should we do about it now? Should I check in the optimization with this comparison fixed to use the system classloader for now? And think about improving this afterwards? I really would like to explore the option of adding a field to ClassLoader a little more (and hear other people's opinions about that). Me too. /Roman
Re: [cp-patches] RFC: File/VMFile improvement
Hi there again, What about this pending patch? I didn't quite understand the discussion around it. IMO the file handling is very platform dependend and while we could handle this generically, I find this path very inconvenient, as it clutters the code, where it could be easy, and it certainly isn't very good for efficiency. I agree that this is not necessarily VM dependent, but instead platform dependend? But I don't think that we should introduce some kind of platform-layer that complements the VM layer in some way. As Jeroen already pointed out, often times the VM is somewhat tied to the platform(s) it supports and thus can just as well provide a reasonable implementation of file handling. As for the VMBlahSocketImpl, I would very much like to have this mess cleaned up (I know, I introduced this in the first place). I came to think of that we don't really need this anyway, as the specs already have a way of abstracting this stuff out (by defining the SocketImpl abstract classes) and we simply added another unnecessary layer here. So may I commit this change to the VMFile class for now? Or should we go another way of splitting out platform dependend stuff? /Roman Roman Kennke schrieb: Jeroen Frijters schrieb: Hi Roman, The attached patch file is empty. Ugh. My bad. Here it is. /Roman Index: vm/reference/java/io/VMFile.java === RCS file: /cvsroot/classpath/classpath/vm/reference/java/io/VMFile.java,v retrieving revision 1.8 diff -u -1 -2 -r1.8 VMFile.java --- vm/reference/java/io/VMFile.java7 Jun 2006 15:09:40 - 1.8 +++ vm/reference/java/io/VMFile.java15 Aug 2006 15:41:49 - @@ -29,24 +29,27 @@ modules, and to copy and distribute the resulting executable under terms of your choice, provided that you also meet, for each linked independent module, the terms and conditions of the license of that module. An independent module is a module which is not derived from or based on this library. If you modify this library, you may extend this exception to your version of the library, but you are not obligated to do so. If you do not wish to do so, delete this exception statement from your version. */ package java.io; +import java.net.MalformedURLException; +import java.net.URL; + import gnu.classpath.Configuration; import gnu.java.io.PlatformHelper; /** * @author Michael Koch ([EMAIL PROTECTED]) */ final class VMFile { // FIXME: We support only case sensitive filesystems currently. static final boolean IS_CASE_SENSITIVE = true; static final boolean IS_DOS_8_3 = false; @@ -200,24 +203,126 @@ { int pos = PlatformHelper.lastIndexOfSeparator(path); if (pos == -1) return path; if (PlatformHelper.endWithSeparator(path)) return ; return path.substring(pos + File.separator.length()); } /** + * Returns the path as an absolute path name. The value returned is the + * current directory plus the separatory string plus the path of the file. + * The current directory is determined from the codeuser.dir/code system + * property. + * + * @param path the path to convert to absolute path + * + * @return the absolute path that corresponds to codepath/code + */ + static String getAbsolutePath(String path) + { +if (File.separatorChar == '\\' + path.length() 0 path.charAt (0) == '\\') + { +// On Windows, even if the path starts with a '\\' it is not +// really absolute until we prefix the drive specifier from +// the current working directory to it. +return System.getProperty (user.dir).substring (0, 2) + path; + } +else if (File.separatorChar == '\\' + path.length() 1 path.charAt (1) == ':' + ((path.charAt (0) = 'a' path.charAt (0) = 'z') + || (path.charAt (0) = 'A' path.charAt (0) = 'Z'))) + { +// On Windows, a process has a current working directory for +// each drive and a path like G:foo\bar would mean the +// absolute path G:\wombat\foo\bar if \wombat is the +// working directory on the G drive. +String drvDir = null; +try + { +drvDir = new File (path.substring (0, 2)).getCanonicalPath(); + } +catch (IOException e) + { +drvDir = path.substring (0, 2) + \\; + } + +// Note: this would return C:\\. for the path C:., if \ +// is the working folder on the C drive, but this is +// consistent with what Sun's JRE 1.4.1.01 actually returns! +if (path.length() 2) + return drvDir + '\\' + path.substring (2, path.length()); +else + return drvDir; + } +else if (path.equals()) + return System.getProperty (user.dir); +
RE: [cp-patches] RFC: Object de-/serialization improvement
Roman Kennke wrote: Jeroen Frijters schrieb: Roman Kennke wrote: What about || (l == VMClassLoader.getSystemClassLoader()) instead ? You shouldn't call VMClassLoader.getSystemClassLoader() (it creates a new instance of the system class loader), but apart from that comparing against the system class loader would work, but I'd much rather see a solution that allows caching for other classes as well. So what should we do about it now? Should I check in the optimization with this comparison fixed to use the system classloader for now? And think about improving this afterwards? Sure. I really would like to explore the option of adding a field to ClassLoader a little more (and hear other people's opinions about that). Me too. I'll cook up a proposal. Regards, Jeroen
RE: [cp-patches] RFC: File/VMFile improvement
Roman Kennke wrote: What about this pending patch? I certainly would like to see it go. Regards, Jeroen
Re: [cp-patches] RFC: File/VMFile improvement
Hi, Jeroen Frijters schrieb: Roman Kennke wrote: What about this pending patch? I certainly would like to see it go. Ok. Dalibor also expressed that he'd like to see it in. So here it goes. 2006-08-21 Roman Kennke [EMAIL PROTECTED] * java/io/File.java (getAbsolutePath): Fetch absolute path from VMFile.getAbsolutePath(). Moved actual impl to there. (isAbsolute): Let VMFile determine the absoluteness. (toURL): Let VMFile convert the filename. * vm/reference/java/io/VMFile.java (getAbsolutePath): New method. (isAbsolute): New method. (toURL): New method. /Roman Index: java/io/File.java === RCS file: /cvsroot/classpath/classpath/java/io/File.java,v retrieving revision 1.65 diff -u -1 -2 -r1.65 File.java --- java/io/File.java 29 Jun 2006 09:59:57 - 1.65 +++ java/io/File.java 21 Aug 2006 09:21:14 - @@ -418,63 +418,26 @@ * This method returns the path of this file as an absolute path name. * If the path name is already absolute, then it is returned. Otherwise * the value returned is the current directory plus the separatory * string plus the path of the file. The current directory is determined * from the codeuser.dir/code system property. * * @return The absolute path of this file */ public String getAbsolutePath() { if (isAbsolute()) return path; -else if (separatorChar == '\\' - path.length() 0 path.charAt (0) == '\\') - { -// On Windows, even if the path starts with a '\\' it is not -// really absolute until we prefix the drive specifier from -// the current working directory to it. -return System.getProperty (user.dir).substring (0, 2) + path; - } -else if (separatorChar == '\\' - path.length() 1 path.charAt (1) == ':' - ((path.charAt (0) = 'a' path.charAt (0) = 'z') - || (path.charAt (0) = 'A' path.charAt (0) = 'Z'))) - { -// On Windows, a process has a current working directory for -// each drive and a path like G:foo\bar would mean the -// absolute path G:\wombat\foo\bar if \wombat is the -// working directory on the G drive. -String drvDir = null; -try - { -drvDir = new File (path.substring (0, 2)).getCanonicalPath(); - } -catch (IOException e) - { -drvDir = path.substring (0, 2) + \\; - } - -// Note: this would return C:\\. for the path C:., if \ -// is the working folder on the C drive, but this is -// consistent with what Sun's JRE 1.4.1.01 actually returns! -if (path.length() 2) - return drvDir + '\\' + path.substring (2, path.length()); -else - return drvDir; - } -else if (path.equals()) - return System.getProperty (user.dir); else - return System.getProperty (user.dir) + separatorChar + path; + return VMFile.getAbsolutePath(path); } /** * This method returns a codeFile/code object representing the * absolute path of this object. * * @return A codeFile/code with the absolute path of the object. * * @since 1.2 */ public File getAbsoluteFile() { @@ -648,33 +611,25 @@ /** * This method returns true if this object represents an absolute file * path and false if it does not. The definition of an absolute path varies * by system. As an example, on GNU systems, a path is absolute if it starts * with a /. * * @return codetrue/code if this object represents an absolute * file name, codefalse/code otherwise. */ public boolean isAbsolute() { -if (separatorChar == '\\') - return path.startsWith(dupSeparator) || - (path.length() 2 - ((path.charAt(0) = 'a' path.charAt(0) = 'z') || - (path.charAt(0) = 'A' path.charAt(0) = 'Z')) - path.charAt(1) == ':' - path.charAt(2) == '\\'); -else - return path.startsWith(separator); +return VMFile.isAbsolute(path); } /** * This method tests whether or not the file represented by this object * is a directory. In order for this method to return codetrue/code, * the file represented by this object must exist and be a directory. * * @return codetrue/code if this file is a directory, codefalse/code * otherwise * * @exception SecurityException If reading of the file is not permitted */ @@ -989,32 +944,25 @@ /** * This method returns a codeURL/code with the codefile:/code * protocol that represents this file. The exact form of this URL is * system dependent. * * @return A codeURL/code for this object. * * @exception MalformedURLException If the URL cannot be created * successfully. */ public URL toURL() throws MalformedURLException
[cp-patches] FYI: InputStreamReader improvement
I'm committing this on behalf of Ingo. He implemented a caching mechanism in InputStreamReader to avoid allocation of new byte[] on every call of read(byte[],int,int). 2006-08-21 Ingo Proetel [EMAIL PROTECTED] * java/io/InputStreamReader.java (bytesCache): New field. (cacheLock): New field. (read(byte[],int,int): Avoid allocations of new byte array on every call and reuse cached byte array if possible. /Roman Index: java/io/InputStreamReader.java === RCS file: /cvsroot/classpath/classpath/java/io/InputStreamReader.java,v retrieving revision 1.29 diff -u -1 -2 -r1.29 InputStreamReader.java --- java/io/InputStreamReader.java 12 Feb 2006 09:36:21 - 1.29 +++ java/io/InputStreamReader.java 21 Aug 2006 10:00:34 - @@ -126,24 +126,34 @@ * java.io canonical name of the encoding. */ private String encoding; /** * We might decode to a 2-char UTF-16 surrogate, which won't fit in the * output buffer. In this case we need to save the surrogate char. */ private char savedSurrogate; private boolean hasSavedSurrogate = false; /** + * A byte array to be reused in read(byte[], int, int). + */ + private byte[] bytesCache; + + /** + * Locks the bytesCache above in read(byte[], int, int). + */ + private Object cacheLock = new Object(); + + /** * This method initializes a new instance of codeInputStreamReader/code * to read from the specified stream using the default encoding. * * @param in The codeInputStream/code to read from */ public InputStreamReader(InputStream in) { if (in == null) throw new NullPointerException(); this.in = in; try { @@ -346,27 +356,37 @@ * @param length The requested number of characters to read. * * @return The actual number of characters read, or -1 if end of stream. * * @exception IOException If an error occurs */ public int read(char[] buf, int offset, int length) throws IOException { if (in == null) throw new IOException(Reader has been closed); if (isDone) return -1; -if(decoder != null){ - int totalBytes = (int)((double)length * maxBytesPerChar); - byte[] bytes = new byte[totalBytes]; +if(decoder != null) + { + int totalBytes = (int)((double) length * maxBytesPerChar); + byte[] bytes; +// Fetch cached bytes array if available and big enough. +synchronized(cacheLock) + { +bytes = bytesCache; +if (bytes == null || bytes.length totalBytes) + bytes = new byte[totalBytes]; +else + bytesCache = null; + } int remaining = 0; if(byteBuffer != null) { remaining = byteBuffer.remaining(); byteBuffer.get(bytes, 0, remaining); } int read; if(totalBytes - remaining 0) { read = in.read(bytes, remaining, totalBytes - remaining); if(read == -1){ @@ -401,30 +421,58 @@ isDone = false; } } if(byteBuffer.hasRemaining()) { byteBuffer.compact(); byteBuffer.flip(); isDone = false; } else byteBuffer = null; read = cb.position() - startPos; - return (read = 0) ? -1 : read; -} else { - byte[] bytes = new byte[length]; + +// Put cached bytes array back if we are finished and the cache +// is null or smaller than the used bytes array. +synchronized (cacheLock) + { +if (byteBuffer == null + (bytesCache == null || bytesCache.length bytes.length)) + bytesCache = bytes; + } +return (read = 0) ? -1 : read; + } +else + { + byte[] bytes; +// Fetch cached bytes array if available and big enough. +synchronized (cacheLock) + { +bytes = bytesCache; +if (bytes == null || length bytes.length) + bytes = new byte[length]; +else + bytesCache = null; + } + int read = in.read(bytes); for(int i=0;iread;i++) buf[offset+i] = (char)(bytes[i]0xFF); + +// Put back byte array into cache if appropriate. +synchronized (cacheLock) + { +if (bytesCache == null || bytesCache.length bytes.length) + bytesCache = bytes; + } return read; } } /** * Reads an char from the input stream and returns it * as an int in the range of 0-65535. This method also will return -1 if * the end of the stream has been reached. * p * This method will block until the char can be read. * * @return The char read or -1 if end of stream
[cp-patches] FYI: CSSScanner cleanup
This changes the CSSScanner to compare the ints directly, rather then casting to char everytime. Now it only casts to char when putting the char into the char[] buffer. 2006-08-21 Roman Kennke [EMAIL PROTECTED] * gnu/javax/swing/text/html/css/CSSScanner.java (main): Use buffered input stream. (nextToken): Removed 65536 workaround. Use int value directly without cast to char. (readComment): Use int value directly without cast to char. Cast to char only when putting the character into the buffer. (readEscape): Likewise. (readIdent): Likewise. (readName): Likewise. (readNum): Likewise. (readString): Likewise. (readWhitespace): Likewise. /Roman Index: gnu/javax/swing/text/html/css/CSSScanner.java === RCS file: /cvsroot/classpath/classpath/gnu/javax/swing/text/html/css/CSSScanner.java,v retrieving revision 1.2 diff -u -1 -2 -r1.2 CSSScanner.java --- gnu/javax/swing/text/html/css/CSSScanner.java 18 Aug 2006 20:22:35 - 1.2 +++ gnu/javax/swing/text/html/css/CSSScanner.java 21 Aug 2006 10:06:56 - @@ -29,24 +29,25 @@ modules, and to copy and distribute the resulting executable under terms of your choice, provided that you also meet, for each linked independent module, the terms and conditions of the license of that module. An independent module is a module which is not derived from or based on this library. If you modify this library, you may extend this exception to your version of the library, but you are not obligated to do so. If you do not wish to do so, delete this exception statement from your version. */ package gnu.javax.swing.text.html.css; +import java.io.BufferedInputStream; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.Reader; /** * A parser for CSS stylesheets. This is based on the grammar from: * * http://www.w3.org/TR/CSS21/syndata.html * * @author Roman Kennke ([EMAIL PROTECTED]) */ @@ -112,226 +113,222 @@ * Fetches the next token. The actual character data is in the parseBuffer * afterwards with the tokenStart at index 0 and the tokenEnd field * pointing to the end of the token. * * @return the next token */ int nextToken() throws IOException { tokenEnd = 0; int token = -1; int next = read(); -if (next == 65535) - System.err.println(BUG); -if (next != -1 next != 65535) // Workaround. +if (next != -1) { -char ch = (char) next; -switch (ch) +switch (next) { case ';': -parseBuffer[0] = ch; +parseBuffer[0] = (char) next; tokenEnd = 1; token = SEMICOLON; break; case '{': -parseBuffer[0] = ch; +parseBuffer[0] = (char) next; tokenEnd = 1; token = CURLY_LEFT; break; case '}': -parseBuffer[0] = ch; +parseBuffer[0] = (char) next; tokenEnd = 1; token = CURLY_RIGHT; break; case '(': -parseBuffer[0] = ch; +parseBuffer[0] = (char) next; tokenEnd = 1; token = PAREN_LEFT; break; case ')': -parseBuffer[0] = ch; +parseBuffer[0] = (char) next; tokenEnd = 1; token = PAREN_RIGHT; break; case '[': -parseBuffer[0] = ch; +parseBuffer[0] = (char) next; tokenEnd = 1; token = BRACE_LEFT; break; case ']': -parseBuffer[0] = ch; +parseBuffer[0] = (char) next; tokenEnd = 1; token = BRACE_RIGHT; break; case '@': -parseBuffer[0] = ch; +parseBuffer[0] = (char) next; tokenEnd = 1; readIdent(); token = ATKEYWORD; break; case '#': -parseBuffer[0] = ch; +parseBuffer[0] = (char) next; tokenEnd = 1; readName(); token = HASH; break; case '\'': case '': -lookahead[0] = ch; +lookahead[0] = next; readString(); token = STRING; break; case ' ': case '\t': case '\r': case '\n': case '\f': -lookahead[0] = ch; +lookahead[0] = next; readWhitespace(); token = S; break; // FIXME: Detecting an URI involves several characters lookahead. // case 'u': //lookahead[0] = ch; //readURI(); //token = URI; //break; case '':
[cp-patches] FYI: Fixed java.io.File normalizePath bug
Hi, I committed the attached patch to fix the handling of paths consisting only of a double separator character. Our normalization would previously turn this into an empty string, instead of a single separator character. This happened both on Windows and on *nix. Regards, Jeroen 2006-08-21 Jeroen Frijters [EMAIL PROTECTED] * java/io/File.java (normalizePath): Fixed handling of // and \\. Index: java/io/File.java === RCS file: /cvsroot/classpath/classpath/java/io/File.java,v retrieving revision 1.66 diff -u -r1.66 File.java --- java/io/File.java 21 Aug 2006 09:23:01 - 1.66 +++ java/io/File.java 21 Aug 2006 12:46:42 - @@ -286,7 +286,8 @@ // example, is a valid and minimal path). if (plen 1 p.charAt (plen - 1) == separatorChar) { - if (! (separatorChar == '\\' plen == 3 p.charAt (1) == ':')) + if (! (separatorChar == '\\' ((plen == 3 p.charAt(1) == ':') +|| (plen == 2 p.charAt(0) == separatorChar return p.substring (0, plen - 1); } else @@ -303,7 +304,16 @@ { dupIndex++; if (dupIndex == plen) - return newpath.toString(); + { +if ((separatorChar == '\\' + newpath.length() == 2 + newpath.charAt(1) == ':') +|| (separatorChar != '\\' newpath.length() == 0)) + { +newpath.append(separatorChar); + } + return newpath.toString(); + } } newpath.append(separatorChar); last = dupIndex; @@ -315,7 +325,9 @@ int end; if (plen 1 p.charAt (plen - 1) == separatorChar) { - if (separatorChar == '\\' plen == 3 p.charAt (1) == ':') + if (separatorChar == '\\' + ((plen == 3 p.charAt(1) == ':') +|| (plen == 2 p.charAt(0) == separatorChar))) end = plen; else end = plen - 1;
Re: [cp-patches] RFC: ClassLoader associative cache
Jeroen Frijters wrote: There are several places in our codebase were we need/want to cache stuff that is associated with a class or class loader. Currently these places retain a strong reference to the class loader, which is not correct, because the class loader should be garbage collectable. I cooked up proposal for an API to use for this and implemented it in the three places I could think of off the top my head (serialization, proxy and resource bundle). This is just a first stab, so no documentation and no change log entry yet. Please comment on the idea and/or the API. Dumb question.. why wouldn't it work to just use a WeakHashMap instead of a HashMap in all those places? -Archie __ Archie Cobbs *CTO, Awarix* http://www.awarix.com
RE: [cp-patches] RFC: ClassLoader associative cache
Jeroen Frijters wrote: Archie Cobbs wrote: Dumb question.. why wouldn't it work to just use a WeakHashMap instead of a HashMap in all those places? Not a dumb question at all. That *would* work. The problem is that (at least on some runtimes) WeakHashMap is much more expensive than this approach. Actually, it would not work in all cases. For serialization it would work, because there the key is a Class instance, but for both Proxy and ResourceBundle, the key is more complex, so it wouldn't work to directly use a WeakHashMap (you'd have to add a layer of indirection, effectively keeping a separate cache per class loader and in that case this approach is arguably better). There may be other reasons, Roman also didn't like to use a WeakHashMap, but I didn't ask why. Using a WeakHashMap would also make it a little harder to implement the ResourceBundle LRU policy, but that was not the motivation for this approach. Regards, Jeroen
[cp-patches] RFC: JTree and BasicToolBarUI
Hey, The application JabRef was not even starting on Classpath. This patch enables it to start up. However, it seems to freeze once that happens. It still needs some work. Could someone kindly approve this patch so that I may commit it. Thanks! Tania 2006-08-21 Tania Bento [EMAIL PROTECTED] * javax/swing/plaf/basic/BasicToolBarUI.java: (setBorderToNonRollover): If border == null, then the insets should be used when saving the old border in the hastable. * javax/swing/JTree.java: (setLeadSelectionPath): Need to make sure that path != null when determining if path.equals(oldValue). This avoid a Null Pointer Exception. Index: javax/swing/JTree.java === RCS file: /cvsroot/classpath/classpath/javax/swing/JTree.java,v retrieving revision 1.73 diff -u -r1.73 JTree.java --- javax/swing/JTree.java 13 Aug 2006 20:34:03 - 1.73 +++ javax/swing/JTree.java 21 Aug 2006 18:47:10 - @@ -2355,7 +2355,7 @@ if (selectionModel != null) { TreePath oldValue = selectionModel.getLeadSelectionPath(); -if (path.equals(oldValue)) +if (path != null path.equals(oldValue)) return; // Repaint the previous and current rows with the lead selection path. Index: javax/swing/plaf/basic/BasicToolBarUI.java === RCS file: /cvsroot/classpath/classpath/javax/swing/plaf/basic/BasicToolBarUI.java,v retrieving revision 1.28 diff -u -r1.28 BasicToolBarUI.java --- javax/swing/plaf/basic/BasicToolBarUI.java 26 Jul 2006 13:39:39 - 1.28 +++ javax/swing/plaf/basic/BasicToolBarUI.java 21 Aug 2006 18:47:10 - @@ -898,7 +898,10 @@ b.setRolloverEnabled(false); // Save old border in hashtable. -borders.put(b, b.getBorder()); +if (b.getBorder() == null) + borders.put(b, b.getInsets()); +else + borders.put(b, b.getBorder()); b.setBorder(nonRolloverBorder); }
Re: [cp-patches] FYI: LocalSocket fixlets
Roman Kennke wrote: This fixes two little issues that I had when porting the LocalSocketImpl to JamaicaVM: - It only loads the library dynamically when actually supported by the runtime. - It includes config.h unconditionally. Actually it doesn't seem to make sense to do: #if HAVE_CONFIG_H #include config.h #endif since HAVE_CONFIG_H is defined in config.h. You should leave the #if around the #include (actually, it should be #ifdef, I think). You'll notice that HAVE_CONFIG_H is defined on the gcc command line in Classpath; autoconf/autoheader adds that to your compile-time flags so you know when you can include config.h. If it's biting you that HAVE_CONFIG_H isn't defined, and you need to include config.h, you should define it, then ;-) I think this is the autoconf way: you don't #include or use something unless you have a HAVE_ macro telling you it's OK. I mean, you could very well just use gcc flags at compile time *instead* of config.h; the latter just makes it easy to have lots of conditionals. I also think this is typical GNU practice, so please leave the conditionals in. Thanks.
Re: [cp-patches] Merge NATIVE-LAYER to HEAD
On Sun, 2006-08-20 at 18:12 -0700, Casey Marshall wrote: Since this finally cleans up the target layer stuff I would like this to go in now and then we build on this for your further improvements to NIO and the socket timeouts. OK? OK. Since I'm working off of cvs head I'm probably queering up the diffs and history; is there any issue with me overwiting some of these changes, not necessarily building off of them? No, certainly not. Just do make sure to test early and often :) For timeouts you might want to look at the new failing omg/corba tests that seem to happen with guilhem's rewrite. Those are regressions, so we should fix them. Since this cleans up stuff beyond javanet, I'd like to see it committed. OK, committed. Cheers, Mark signature.asc Description: This is a digitally signed message part
Re: [cp-patches] RFC: JTree and BasicToolBarUI
Hey Tania, On Mon, 2006-08-21 at 15:00 -0400, Tania Bento wrote: The application JabRef was not even starting on Classpath. This patch enables it to start up. However, it seems to freeze once that happens. It still needs some work. Cool. CCed Egon who was interested JabRef. Could one of you update the JabRef entry on http://developer.classpath.org/mediation/FreeSwingTestApps 2006-08-21 Tania Bento [EMAIL PROTECTED] * javax/swing/plaf/basic/BasicToolBarUI.java: (setBorderToNonRollover): If border == null, then the insets should be used when saving the old border in the hastable. * javax/swing/JTree.java: (setLeadSelectionPath): Need to make sure that path != null when determining if path.equals(oldValue). This avoid a Null Pointer Exception. Maybe that check needs to be if (path == oldValue || path != null path.equals(oldValue)) to catch the case both path and oldValue are null? Thanks, Mark
Re: [cp-patches] RFC: File/VMFile improvement
Roman Kennke wrote: Hi there again, What about this pending patch? I didn't quite understand the discussion around it. IMO the file handling is very platform dependend and while we could handle this generically, I find this path very inconvenient, as it clutters the code, where it could be easy, and it certainly isn't very good for efficiency. I agree that this is not necessarily VM dependent, but instead platform dependend? But I don't think that we should introduce some kind of platform-layer that complements the VM layer in some way. As Jeroen already pointed out, often times the VM is somewhat tied to the platform(s) it supports and thus can just as well provide a reasonable implementation of file handling. Sorry, that was kind of a bikeshed issue. VM-vs-platform isn't *that* big a deal, it just feels clumsy, and has been used as an I don't understand this umbrella in the past. As for the VMBlahSocketImpl, I would very much like to have this mess cleaned up (I know, I introduced this in the first place). I came to think of that we don't really need this anyway, as the specs already have a way of abstracting this stuff out (by defining the SocketImpl abstract classes) and we simply added another unnecessary layer here. This is true, and it's the same with java.nio (it's essentially a service provider interface through the SelectorProvider class). There is *some* value in isolating these platform-specific bits as much as possible (and keeping them, and the amount of JNI code, to a minimum!), and making porting Classpath a low-barrier-to-entry. What, if any, performance tradeoffs there are is a good question, however. So may I commit this change to the VMFile class for now? Or should we go another way of splitting out platform dependend stuff? Sorry for not looking at it. The patch looks reasonable to me. Real code is always preferable to me bickering about vaporware :-)