You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinson Lee <vl...@freedesktop.org> on 2013/08/27 07:11:34 UTC
Review Request 13840: stout: Fix clang return-type build error.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13840/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Eric Biederman, Ian Downes, and David Mackey.
Repository: mesos-git
Description
-------
Author: Vinson Lee <vl...@twitter.com>
stout: Fix clang return-type build error.
This patch fixes this build error with clang.
In file included from sched/sched.cpp:35:
In file included from ../3rdparty/libprocess/include/process/delay.hpp:6:
In file included from ../3rdparty/libprocess/include/process/dispatch.hpp:9:
In file included from ../3rdparty/libprocess/include/process/process.hpp:13:
In file included from ../3rdparty/libprocess/include/process/event.hpp:10:
In file included from ../3rdparty/libprocess/include/process/socket.hpp:10:
In file included from ../3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp:65:
../3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp:46:1: error: control may reach end of non-void function [-Werror,-Wreturn-type]
}
^
Signed-off-by: Vinson Lee <vl...@twitter.com>
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp b41ba63
Diff: https://reviews.apache.org/r/13840/diff/
Testing
-------
Thanks,
Vinson Lee
Re: Review Request 13840: stout: Fix clang return-type build error.
Posted by Vinson Lee <vl...@freedesktop.org>.
> On Aug. 27, 2013, 7:31 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp, lines 29-34
> > <https://reviews.apache.org/r/13840/diff/1/?file=345051#file345051line29>
> >
> > How about the following so that we can avoid needing to assert:
> >
> > ssize_t result;
> >
> > suppress (SIGPIPE) {
> > // This will set errno to EPIPE if a SIGPIPE occurs.
> > result = ::sendfile(s, fd, &offset, length);
> > }
> >
> > return result;
The return value will still be undefined if suppress(SIGPIPE) fails.
- Vinson
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13840/#review25632
-----------------------------------------------------------
On Aug. 27, 2013, 5:11 a.m., Vinson Lee wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13840/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2013, 5:11 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Eric Biederman, Ian Downes, and David Mackey.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Author: Vinson Lee <vl...@twitter.com>
>
> stout: Fix clang return-type build error.
>
> This patch fixes this build error with clang.
>
> In file included from sched/sched.cpp:35:
> In file included from ../3rdparty/libprocess/include/process/delay.hpp:6:
> In file included from ../3rdparty/libprocess/include/process/dispatch.hpp:9:
> In file included from ../3rdparty/libprocess/include/process/process.hpp:13:
> In file included from ../3rdparty/libprocess/include/process/event.hpp:10:
> In file included from ../3rdparty/libprocess/include/process/socket.hpp:10:
> In file included from ../3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp:65:
> ../3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp:46:1: error: control may reach end of non-void function [-Werror,-Wreturn-type]
> }
> ^
>
> Signed-off-by: Vinson Lee <vl...@twitter.com>
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp b41ba63
>
> Diff: https://reviews.apache.org/r/13840/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vinson Lee
>
>
Re: Review Request 13840: stout: Fix clang return-type build error.
Posted by Ben Mahler <be...@gmail.com>.
> On Aug. 27, 2013, 7:31 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp, lines 29-34
> > <https://reviews.apache.org/r/13840/diff/1/?file=345051#file345051line29>
> >
> > How about the following so that we can avoid needing to assert:
> >
> > ssize_t result;
> >
> > suppress (SIGPIPE) {
> > // This will set errno to EPIPE if a SIGPIPE occurs.
> > result = ::sendfile(s, fd, &offset, length);
> > }
> >
> > return result;
>
> Vinson Lee wrote:
> The return value will still be undefined if suppress(SIGPIPE) fails.
suppress is a macro that guarantees the code in the suppressed scope will be executed.
I agree that we should guard against a regression but I'm concerned about using assert because it can be disabled. Can you use fatal (from fatal.hpp)?
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13840/#review25632
-----------------------------------------------------------
On Aug. 27, 2013, 5:11 a.m., Vinson Lee wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13840/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2013, 5:11 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Eric Biederman, Ian Downes, and David Mackey.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Author: Vinson Lee <vl...@twitter.com>
>
> stout: Fix clang return-type build error.
>
> This patch fixes this build error with clang.
>
> In file included from sched/sched.cpp:35:
> In file included from ../3rdparty/libprocess/include/process/delay.hpp:6:
> In file included from ../3rdparty/libprocess/include/process/dispatch.hpp:9:
> In file included from ../3rdparty/libprocess/include/process/process.hpp:13:
> In file included from ../3rdparty/libprocess/include/process/event.hpp:10:
> In file included from ../3rdparty/libprocess/include/process/socket.hpp:10:
> In file included from ../3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp:65:
> ../3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp:46:1: error: control may reach end of non-void function [-Werror,-Wreturn-type]
> }
> ^
>
> Signed-off-by: Vinson Lee <vl...@twitter.com>
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp b41ba63
>
> Diff: https://reviews.apache.org/r/13840/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vinson Lee
>
>
Re: Review Request 13840: stout: Fix clang return-type build error.
Posted by Vinson Lee <vl...@freedesktop.org>.
> On Aug. 27, 2013, 7:31 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp, lines 29-34
> > <https://reviews.apache.org/r/13840/diff/1/?file=345051#file345051line29>
> >
> > How about the following so that we can avoid needing to assert:
> >
> > ssize_t result;
> >
> > suppress (SIGPIPE) {
> > // This will set errno to EPIPE if a SIGPIPE occurs.
> > result = ::sendfile(s, fd, &offset, length);
> > }
> >
> > return result;
>
> Vinson Lee wrote:
> The return value will still be undefined if suppress(SIGPIPE) fails.
>
> Ben Mahler wrote:
> suppress is a macro that guarantees the code in the suppressed scope will be executed.
>
> I agree that we should guard against a regression but I'm concerned about using assert because it can be disabled. Can you use fatal (from fatal.hpp)?
Ok. I don't have a strong opinion either way. The actual build error is well understood now, so feel free to use any approach that fixes the build error.
- Vinson
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13840/#review25632
-----------------------------------------------------------
On Aug. 27, 2013, 5:11 a.m., Vinson Lee wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13840/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2013, 5:11 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Eric Biederman, Ian Downes, and David Mackey.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Author: Vinson Lee <vl...@twitter.com>
>
> stout: Fix clang return-type build error.
>
> This patch fixes this build error with clang.
>
> In file included from sched/sched.cpp:35:
> In file included from ../3rdparty/libprocess/include/process/delay.hpp:6:
> In file included from ../3rdparty/libprocess/include/process/dispatch.hpp:9:
> In file included from ../3rdparty/libprocess/include/process/process.hpp:13:
> In file included from ../3rdparty/libprocess/include/process/event.hpp:10:
> In file included from ../3rdparty/libprocess/include/process/socket.hpp:10:
> In file included from ../3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp:65:
> ../3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp:46:1: error: control may reach end of non-void function [-Werror,-Wreturn-type]
> }
> ^
>
> Signed-off-by: Vinson Lee <vl...@twitter.com>
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp b41ba63
>
> Diff: https://reviews.apache.org/r/13840/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vinson Lee
>
>
Re: Review Request 13840: stout: Fix clang return-type build error.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13840/#review25632
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp
<https://reviews.apache.org/r/13840/#comment50118>
How about the following so that we can avoid needing to assert:
ssize_t result;
suppress (SIGPIPE) {
// This will set errno to EPIPE if a SIGPIPE occurs.
result = ::sendfile(s, fd, &offset, length);
}
return result;
- Ben Mahler
On Aug. 27, 2013, 5:11 a.m., Vinson Lee wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13840/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2013, 5:11 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Eric Biederman, Ian Downes, and David Mackey.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Author: Vinson Lee <vl...@twitter.com>
>
> stout: Fix clang return-type build error.
>
> This patch fixes this build error with clang.
>
> In file included from sched/sched.cpp:35:
> In file included from ../3rdparty/libprocess/include/process/delay.hpp:6:
> In file included from ../3rdparty/libprocess/include/process/dispatch.hpp:9:
> In file included from ../3rdparty/libprocess/include/process/process.hpp:13:
> In file included from ../3rdparty/libprocess/include/process/event.hpp:10:
> In file included from ../3rdparty/libprocess/include/process/socket.hpp:10:
> In file included from ../3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp:65:
> ../3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp:46:1: error: control may reach end of non-void function [-Werror,-Wreturn-type]
> }
> ^
>
> Signed-off-by: Vinson Lee <vl...@twitter.com>
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp b41ba63
>
> Diff: https://reviews.apache.org/r/13840/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vinson Lee
>
>
Re: Review Request 13840: stout: Fix clang return-type build error.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13840/#review25776
-----------------------------------------------------------
Ship it!
Sorry for the run-around here, I've committed this with a small change to fix the include order. Please mark as submitted, thanks Vinson!
- Ben Mahler
On Aug. 28, 2013, 12:26 a.m., Vinson Lee wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13840/
> -----------------------------------------------------------
>
> (Updated Aug. 28, 2013, 12:26 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Eric Biederman, Ian Downes, and David Mackey.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Author: Vinson Lee <vl...@twitter.com>
>
> stout: Fix clang return-type build error.
>
> This patch fixes this build error with clang.
>
> In file included from sched/sched.cpp:35:
> In file included from ../3rdparty/libprocess/include/process/delay.hpp:6:
> In file included from ../3rdparty/libprocess/include/process/dispatch.hpp:9:
> In file included from ../3rdparty/libprocess/include/process/process.hpp:13:
> In file included from ../3rdparty/libprocess/include/process/event.hpp:10:
> In file included from ../3rdparty/libprocess/include/process/socket.hpp:10:
> In file included from ../3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp:65:
> ../3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp:46:1: error: control may reach end of non-void function [-Werror,-Wreturn-type]
> }
> ^
>
> Signed-off-by: Vinson Lee <vl...@twitter.com>
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp b41ba63
>
> Diff: https://reviews.apache.org/r/13840/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vinson Lee
>
>
Re: Review Request 13840: stout: Fix clang return-type build error.
Posted by Vinson Lee <vl...@freedesktop.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13840/
-----------------------------------------------------------
(Updated Aug. 28, 2013, 12:26 a.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Chi Zhang, Eric Biederman, Ian Downes, and David Mackey.
Changes
-------
Per Ben Mahler's comments, replace assert with fatal.
Repository: mesos-git
Description
-------
Author: Vinson Lee <vl...@twitter.com>
stout: Fix clang return-type build error.
This patch fixes this build error with clang.
In file included from sched/sched.cpp:35:
In file included from ../3rdparty/libprocess/include/process/delay.hpp:6:
In file included from ../3rdparty/libprocess/include/process/dispatch.hpp:9:
In file included from ../3rdparty/libprocess/include/process/process.hpp:13:
In file included from ../3rdparty/libprocess/include/process/event.hpp:10:
In file included from ../3rdparty/libprocess/include/process/socket.hpp:10:
In file included from ../3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp:65:
../3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp:46:1: error: control may reach end of non-void function [-Werror,-Wreturn-type]
}
^
Signed-off-by: Vinson Lee <vl...@twitter.com>
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp b41ba63
Diff: https://reviews.apache.org/r/13840/diff/
Testing
-------
Thanks,
Vinson Lee