Linux: Fix a bunch of clang errors from not handling return values.

Linux: Fix a bunch of clang errors from not handling return values.
Committed: https://code.google.com/p/google-breakpad/source/detail?r=995

[Lei Zhang (chromium)]

See http://code.google.com/p/chromium/issues/detail?id=138571

[Mark Mentovai]

https://breakpad.appspot.com/421002/diff/2001/src/client/linux/crash_generati...
File src/client/linux/crash_generation/crash_generation_client.cc (right):

https://breakpad.appspot.com/421002/diff/2001/src/client/linux/crash_generati...
src/client/linux/crash_generation/crash_generation_client.cc:71: bool ret =
HANDLE_EINTR(sys_sendmsg(server_fd_, &msg, 0));
bool here and <= 0 two lines later? Is that really the pattern you want to use?
That’s infuriatingly obtuse.

[Lei Zhang (chromium)]

https://breakpad.appspot.com/421002/diff/2001/src/client/linux/crash_generati...
File src/client/linux/crash_generation/crash_generation_client.cc (right):

https://breakpad.appspot.com/421002/diff/2001/src/client/linux/crash_generati...
src/client/linux/crash_generation/crash_generation_client.cc:71: bool ret =
HANDLE_EINTR(sys_sendmsg(server_fd_, &msg, 0));
On 2012/07/23 22:03:16, Mark Mentovai wrote:
> bool here and <= 0 two lines later? Is that really the pattern you want to
use?
> That’s infuriatingly obtuse.

Do you prefer this instead? I don't have strong feelings either way.

if (foo <= 0) {
  sys_close(fds[1]);
  return false;
}
sys_close(fds[1]);

Is there another alternative pattern that I'm forgetting?

[Mark Mentovai]

https://breakpad.appspot.com/421002/diff/2001/src/client/linux/crash_generati...
File src/client/linux/crash_generation/crash_generation_client.cc (right):

https://breakpad.appspot.com/421002/diff/2001/src/client/linux/crash_generati...
src/client/linux/crash_generation/crash_generation_client.cc:71: bool ret =
HANDLE_EINTR(sys_sendmsg(server_fd_, &msg, 0));
thestig wrote:
> On 2012/07/23 22:03:16, Mark Mentovai wrote:
> > bool here and <= 0 two lines later? Is that really the pattern you want to
> use?
> > That’s infuriatingly obtuse.
> 
> Do you prefer this instead? I don't have strong feelings either way.
> 
> if (foo <= 0) {
>   sys_close(fds[1]);
>   return false;
> }
> sys_close(fds[1]);
> 
> Is there another alternative pattern that I'm forgetting?

I meant…

is bool really the return type of sys_sendmsg?

and

why are you testing a_bool <= 0?

[Lei Zhang (chromium)]

https://breakpad.appspot.com/421002/diff/2001/src/client/linux/crash_generati...
File src/client/linux/crash_generation/crash_generation_client.cc (right):

https://breakpad.appspot.com/421002/diff/2001/src/client/linux/crash_generati...
src/client/linux/crash_generation/crash_generation_client.cc:71: bool ret =
HANDLE_EINTR(sys_sendmsg(server_fd_, &msg, 0));
On 2012/07/23 22:31:06, Mark Mentovai wrote:
> thestig wrote:
> > On 2012/07/23 22:03:16, Mark Mentovai wrote:
> > > bool here and <= 0 two lines later? Is that really the pattern you want to
> > use?
> > > That’s infuriatingly obtuse.
> > 
> > Do you prefer this instead? I don't have strong feelings either way.
> > 
> > if (foo <= 0) {
> >   sys_close(fds[1]);
> >   return false;
> > }
> > sys_close(fds[1]);
> > 
> > Is there another alternative pattern that I'm forgetting?
> 
> I meant…
> 
> is bool really the return type of sys_sendmsg?
> 
> and
> 
> why are you testing a_bool <= 0?

Clearly a case of the Mondays. s/bool/ssize_t/

[Mark Mentovai]

LGTM otherwise

https://breakpad.appspot.com/421002/diff/6002/src/client/linux/crash_generati...
File src/client/linux/crash_generation/crash_generation_client.cc (right):

https://breakpad.appspot.com/421002/diff/6002/src/client/linux/crash_generati...
src/client/linux/crash_generation/crash_generation_client.cc:71: ssize_t ret =
HANDLE_EINTR(sys_sendmsg(server_fd_, &msg, 0));
Much better!

https://breakpad.appspot.com/421002/diff/6002/src/common/linux/ignore_ret.h
File src/common/linux/ignore_ret.h (right):

https://breakpad.appspot.com/421002/diff/6002/src/common/linux/ignore_ret.h#n...
src/common/linux/ignore_ret.h:34: // where we either a) know the call cannot
fail, or b) there is nothing we
Don’t say “we” in comments.

https://breakpad.appspot.com/421002/diff/6002/src/common/linux/ignore_ret.h#n...
src/common/linux/ignore_ret.h:40: #endif // ifndef COMMON_LINUX_INGORE_RET_H_
Two spaces between #endif and //. See “line comments” at
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Implem...

[Lei Zhang (chromium)]

https://breakpad.appspot.com/421002/diff/6002/src/common/linux/ignore_ret.h
File src/common/linux/ignore_ret.h (right):

https://breakpad.appspot.com/421002/diff/6002/src/common/linux/ignore_ret.h#n...
src/common/linux/ignore_ret.h:34: // where we either a) know the call cannot
fail, or b) there is nothing we
On 2012/07/23 23:21:59, Mark Mentovai wrote:
> Don’t say “we” in comments.

Done.

https://breakpad.appspot.com/421002/diff/6002/src/common/linux/ignore_ret.h#n...
src/common/linux/ignore_ret.h:40: #endif // ifndef COMMON_LINUX_INGORE_RET_H_
On 2012/07/23 23:21:59, Mark Mentovai wrote:
> Two spaces between #endif and //. See “line comments” at
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Implem...

Done. This came from eintr_wrapper.h, so I fixed it there as well.

[Mark Mentovai]

LGTM otherwise

https://breakpad.appspot.com/421002/diff/7002/src/common/linux/eintr_wrapper.h
File src/common/linux/eintr_wrapper.h (right):

https://breakpad.appspot.com/421002/diff/7002/src/common/linux/eintr_wrapper....
src/common/linux/eintr_wrapper.h:47: #endif  // ifndef
COMMON_LINUX_EINTR_WRAPPER_H_
Oh, hmm, we don’t usually write ifndef in the comment either.

https://breakpad.appspot.com/421002/diff/7002/src/common/linux/ignore_ret.h
File src/common/linux/ignore_ret.h (right):

https://breakpad.appspot.com/421002/diff/7002/src/common/linux/ignore_ret.h#n...
src/common/linux/ignore_ret.h:40: #endif  // ifndef COMMON_LINUX_INGORE_RET_H_
Same.

[Lei Zhang (chromium)]

On 2012/07/24 00:08:53, Mark Mentovai wrote:
> LGTM otherwise
> 
> https://breakpad.appspot.com/421002/diff/7002/src/common/linux/eintr_wrapper.h
> File src/common/linux/eintr_wrapper.h (right):
> 
>
https://breakpad.appspot.com/421002/diff/7002/src/common/linux/eintr_wrapper....
> src/common/linux/eintr_wrapper.h:47: #endif  // ifndef
> COMMON_LINUX_EINTR_WRAPPER_H_
> Oh, hmm, we don’t usually write ifndef in the comment either.
> 
> https://breakpad.appspot.com/421002/diff/7002/src/common/linux/ignore_ret.h
> File src/common/linux/ignore_ret.h (right):
> 
>
https://breakpad.appspot.com/421002/diff/7002/src/common/linux/ignore_ret.h#n...
> src/common/linux/ignore_ret.h:40: #endif  // ifndef COMMON_LINUX_INGORE_RET_H_
> Same.

Done and done.