On Jan 8, 2007, at 6:09 PM, Karan Malhi wrote:
Thanks Dain, I will make the suggested changes and submit another patch.
I just have to point out.... Most people tend to say "sorry" when they get feedback, but you said "thanks." Definitely the right attitude to have! Big thumbs up!
Thanks Karan and Dain. -David
On 1/8/07, Dain Sundstrom <[EMAIL PROTECTED]> wrote:This is a good start but I see some bugs :) comments in line... -dain On Jan 7, 2007, at 12:59 PM, [EMAIL PROTECTED] wrote: > + JarInputStream jis = new JarInputStream(new > FileInputStream(jarFile)); > + File tempJar = File.createTempFile("temp", "jar"); The temp file should be created in the same directory as the final original file because on some platforms the temp directory is on a separate drive and rename only works when the source and target file are on the same drive. > + JarOutputStream jos = new JarOutputStream(new > FileOutputStream(tempJar)); > + JarEntry nextJarEntry = null;> + while ((nextJarEntry = jis.getNextJarEntry()) != null) {> + jos.putNextEntry(nextJarEntry); > + } You need to copy the contents from jis to jos after putNextEntry. > + jis.close(); > + jos.putNextEntry(new JarEntry(file)); > + FileInputStream fis = new FileInputStream(file); > + for (int c = fis.read(); c != -1; c = fis.read()) { > + jos.write(c); > + } Byte at a time is quite slow. Instead I suggest using 4k blocks. > + fis.close(); > + jos.close(); Close should be in a finally block. > + File oldJar = new File(jarFile); > + oldJar.delete(); > + tempJar.renameTo(oldJar); It is safer to move the old file aside, rename the temp jar, thendelete the old file. That way if the rename fails, you can back it out.> + } catch (FileNotFoundException e) { > + throw new OpenEJBException(messages.format("file. > 0003", file, jarFile, e.getMessage())); > + } catch (IOException e) { > + throw new OpenEJBException(messages.format("file. > 0003", file, jarFile, e.getMessage())); > } > > }-- Karan Malhi
