You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2013/03/22 05:17:04 UTC
Review Request: Fixed os::mktemp to use mkstemp,
instead of the deprecated mktemp syscall.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10074/
-----------------------------------------------------------
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
Re: Review Request: Fixed os::mktemp to use mkstemp,
instead of the deprecated mktemp syscall.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10074/
-----------------------------------------------------------
(Updated March 22, 2013, 10:16 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Added a comment on the ignored close() result, per vinod's review.
Description
-------
See the mktemp documentation for the warnings.
http://pubs.opengroup.org/onlinepubs/000095399/functions/mktemp.html
Diffs (updated)
-----
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
Re: Review Request: Fixed os::mktemp to use mkstemp,
instead of the deprecated mktemp syscall.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10074/#review18286
-----------------------------------------------------------
Ship it!
Can you add a comment above close() like we do in write?
- Vinod Kone
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
>
>
Re: Review Request: Fixed os::mktemp to use mkstemp,
instead of the deprecated mktemp syscall.
Posted by Ben Mahler <be...@gmail.com>.
> 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
>
>
Re: Review Request: Fixed os::mktemp to use mkstemp,
instead of the deprecated mktemp syscall.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10074/#review18242
-----------------------------------------------------------
third_party/libprocess/third_party/stout/include/stout/os.hpp
<https://reviews.apache.org/r/10074/#comment38454>
check for close error?
- Vinod Kone
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
>
>