On Thu, Feb 6, 2014 at 11:23 AM, sebb <seb...@gmail.com> wrote: > On 6 February 2014 16:06, Gary Gregory <garydgreg...@gmail.com> wrote: > > On Thu, Feb 6, 2014 at 10:18 AM, sebb <seb...@gmail.com> wrote: > > > >> On 6 February 2014 15:04, Gary Gregory <garydgreg...@gmail.com> wrote: > >> > On Thu, Feb 6, 2014 at 9:04 AM, sebb <seb...@gmail.com> wrote: > >> > > >> >> On 6 February 2014 13:53, Gary Gregory <garydgreg...@gmail.com> > wrote: > >> >> > Looking at code like DiskFileItem:684: > >> >> > > >> >> > FileInputStream input = new FileInputStream(dfosFile); > >> >> > IOUtils.copy(input, output); > >> >> > > >> >> > Why is this not: > >> >> > > >> >> > FileInputStream input = new FileInputStream(dfosFile); > >> >> > try { > >> >> > IOUtils.copy(input, output); > >> >> > } finally { > >> >> > input.close(); > >> >> > } > >> >> > > >> >> > ? > >> >> > >> >> Oversight? > >> >> > >> >> The local Streams.copy() methods do close input and optionally > output. > >> >> Maybe it was thought IO did the same? > >> >> > >> >> The close should perhaps be > >> >> > >> >> IOUtils.closeQuietly() > >> >> > >> >> Are we interested in knowibg about input close() failures? > >> >> > >> > > >> > Good question. I would guess 'yes'. If you write to a file and then > >> cannot > >> > close it, you might not have flushed it, so the file could be corrupt? > >> > >> But this is an _input_ file. > >> Are we interested in close failure once the copy has completed? > >> > > > > I would say no. BUT... if you replace the Streams code with: > > > > public static long copy(InputStream inputStream, OutputStream > > outputStream, boolean closeOutputStream, byte[] buffer) > > throws IOException { > > try { > > return IOUtils.copy(inputStream, outputStream); > > } finally { > > IOUtils.closeQuietly(inputStream); > > if (closeOutputStream) { > > IOUtils.closeQuietly(outputStream); > > } > > } > > } > > > > You will get an NPE in IOUtils.copy(inputStream, outputStream). > > > > Because in some cases [fileupload] calls Streams.copy with a null output > > stream. The Javadoc for IOUtils.copy states that this is grounds for an > NPE. > > > > So that's a no-go. I wonder if we should propose to make IOUtils.copy > > ignore null output to accommodate this scenario? > > No. > Change of API behaviour and allowing null will probably hide some bugs. > > However, Streams could potentially call IOUtils. >
Streams now reuses IOUtils.closeQuietly(). Gary > > > Gary > > > >> > >> > Gary > >> > > >> > > >> >> > >> >> > Gary > >> >> > > >> >> > > >> >> > -- > >> >> > E-Mail: garydgreg...@gmail.com | ggreg...@apache.org > >> >> > Java Persistence with Hibernate, Second Edition< > >> >> http://www.manning.com/bauer3/> > >> >> > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> > >> >> > Spring Batch in Action <http://www.manning.com/templier/> > >> >> > Blog: http://garygregory.wordpress.com > >> >> > Home: http://garygregory.com/ > >> >> > Tweet! http://twitter.com/GaryGregory > >> >> > >> >> --------------------------------------------------------------------- > >> >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > >> >> For additional commands, e-mail: dev-h...@commons.apache.org > >> >> > >> >> > >> > > >> > > >> > -- > >> > E-Mail: garydgreg...@gmail.com | ggreg...@apache.org > >> > Java Persistence with Hibernate, Second Edition< > >> http://www.manning.com/bauer3/> > >> > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> > >> > Spring Batch in Action <http://www.manning.com/templier/> > >> > Blog: http://garygregory.wordpress.com > >> > Home: http://garygregory.com/ > >> > Tweet! http://twitter.com/GaryGregory > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > >> For additional commands, e-mail: dev-h...@commons.apache.org > >> > >> > > > > > > -- > > E-Mail: garydgreg...@gmail.com | ggreg...@apache.org > > Java Persistence with Hibernate, Second Edition< > http://www.manning.com/bauer3/> > > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> > > Spring Batch in Action <http://www.manning.com/templier/> > > Blog: http://garygregory.wordpress.com > > Home: http://garygregory.com/ > > Tweet! http://twitter.com/GaryGregory > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > -- E-Mail: garydgreg...@gmail.com | ggreg...@apache.org Java Persistence with Hibernate, Second Edition<http://www.manning.com/bauer3/> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> Spring Batch in Action <http://www.manning.com/templier/> Blog: http://garygregory.wordpress.com Home: http://garygregory.com/ Tweet! http://twitter.com/GaryGregory