Thanks Dain,
I will make the suggested changes and submit another patch.
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, then
delete 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
