> On March 22, 2013, 4:32 a.m., Vinod Kone wrote:
> > third_party/libprocess/third_party/stout/include/stout/os.hpp, line 211
> > <https://reviews.apache.org/r/10074/diff/1/?file=273849#file273849line211>
> >
> >     check for close error?

I think we should ignore any close errors, when considering whether this 
function is succeeding or not:
  EINVAL: mkstemp returned successfully, so this one is not possible
  EINTR: not much we can do here, see: 
http://www.daemonology.net/blog/2011-12-17-POSIX-close-is-broken.html
  EIO: This one is an optional error in POSIX, but since we've been given the 
open file from mkstemp, there should not be I/O errors upon closing it.

I think it's ok to apply the same principle as we do in write():
  // NOTE: We ignore the return value of close(). This is because users calling
  // this function are interested in the return value of write(). Also an
  // unsuccessful close() doesn't affect the write.

Normally, I would consider LOG(ERROR) as ok here, but since this is stout we 
can't do logging.


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10074/#review18242
-----------------------------------------------------------


On March 22, 2013, 4:17 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10074/
> -----------------------------------------------------------
> 
> (Updated March 22, 2013, 4:17 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> See the mktemp documentation for the warnings.
> http://pubs.opengroup.org/onlinepubs/000095399/functions/mktemp.html
> 
> 
> Diffs
> -----
> 
>   third_party/libprocess/third_party/stout/include/stout/os.hpp 
> 905e783f837395dd8d3db16e487845c120ae8c91 
> 
> Diff: https://reviews.apache.org/r/10074/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Reply via email to