gearman/gearmand

Should the '||' on libtest/cmdline.cc:207 be '&&'?

Closed this issue · 2 comments

Just wondering if anyone else thinks this is a typo (see line 207)?

gearmand/libtest/cmdline.cc

Lines 207 to 210 in dd9ac59

#if defined(POSIX_SPAWN_USEVFORK) || defined(__linux__)
// Use USEVFORK on linux
flags |= POSIX_SPAWN_USEVFORK;
#endif

I noticed this patch for compiling gearmand on alpine:3.9 and wondered if it's correct:

https://github.com/artefactual-labs/docker-gearmand/blob/6e3d05f69f7653682cfdcf5bbb9fbb57b4c335f8/1.1.18/patches/libtest-cmdline.cc.patch

Interestingly, this patch for libmemcached changes the line to

#if defined(POSIX_SPAWN_USEVFORK) || defined(__GLIBC__)

instead. I think that's the better way to go. What do you think?

I think it's kind of a moot point since musl (what Alpine uses instead of glibc) has defined POSIX_SPAWN_USEVFORK since 2017. Note that I was able to successfully bootstrap, build, and test gearmand on alpine:latest in PR #296 without this patch, so I guess this only applies to older versions of Alpine. But it's probably worth fixing anyway in case someone wants to do that?

https://git.musl-libc.org/cgit/musl/tree/include/spawn.h
https://git.musl-libc.org/cgit/musl/log/include/spawn.h