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. > 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