| Message ID | 1400227385.10012.98.camel@G3620.my.own.domain |
|---|---|
| State | New |
| Headers | show |
On Fri, May 16, 2014 at 1:03 AM, Svante Signell <svante.signell@gmail.com> wrote: > > For that part of the patch without it the build on GNU/Hurd fails. On > the other hand SYS_FCNTL is not defined for e.g. GNU/Linux either. This > is used in gcc-4.9-4.9.0/src/libgo/go/net/fd_unix.go: > func dupCloseOnExec(fd int) (newfd int, err error) { > if atomic.LoadInt32(&tryDupCloexec) == 1 && syscall.F_DUPFD_CLOEXEC!=0 { > r0, _, e1 := syscall.Syscall(syscall.SYS_FCNTL, uintptr(fd), > syscall.F_DUPFD_CLOEXEC, 0) > > It is yet unknown how the build succeeds on Linux without the SYS_FCNTL > being defined? Maybe any the conditions above are not met. On GNU/Linux SYS_FCNTL is defined by the generated file sysinfo.go, because SYS_fcntl is defined by <syscall.h>. Ian
On Fri, 2014-05-16 at 06:20 -0700, Ian Lance Taylor wrote: > On Fri, May 16, 2014 at 1:03 AM, Svante Signell > <svante.signell@gmail.com> wrote: > > > > For that part of the patch without it the build on GNU/Hurd fails. On > > the other hand SYS_FCNTL is not defined for e.g. GNU/Linux either. This > > is used in gcc-4.9-4.9.0/src/libgo/go/net/fd_unix.go: > > func dupCloseOnExec(fd int) (newfd int, err error) { > > if atomic.LoadInt32(&tryDupCloexec) == 1 && syscall.F_DUPFD_CLOEXEC!=0 { > > r0, _, e1 := syscall.Syscall(syscall.SYS_FCNTL, uintptr(fd), > > syscall.F_DUPFD_CLOEXEC, 0) > > > > It is yet unknown how the build succeeds on Linux without the SYS_FCNTL > > being defined? Maybe any the conditions above are not met. > > On GNU/Linux SYS_FCNTL is defined by the generated file sysinfo.go, > because SYS_fcntl is defined by <syscall.h>. Thanks, then that part of the patch should read: # Special treatment of _SYS_fcntl for GNU/Hurd if ! grep '^const _SYS_fcntl' ${OUT} > /dev/null 2>&1; then echo "const SYS_FCNTL = 0" >> ${OUT} fi Shall I submit a new patch8.diff (or all patches again)?
Svante Signell, le Fri 16 May 2014 10:03:05 +0200, a écrit : > is used in gcc-4.9-4.9.0/src/libgo/go/net/fd_unix.go: > func dupCloseOnExec(fd int) (newfd int, err error) { > if atomic.LoadInt32(&tryDupCloexec) == 1 && syscall.F_DUPFD_CLOEXEC!=0 { > r0, _, e1 := syscall.Syscall(syscall.SYS_FCNTL, uintptr(fd), > syscall.F_DUPFD_CLOEXEC, 0) That code can not work as it is, fcntl is not a system call on GNU/Hurd. Why isn't gccgo just using the C fcntl function? That one will just work and be portable. > +# Special treatment of EWOULDBLOCK for GNU/Hurd > +# /usr/include/bits/errno.h: #define EWOULDBLOCK EAGAIN > +if egrep 'define EWOULDBLOCK EAGAIN' gen-sysinfo.go > /dev/null 2>&1; then > + egrep '^const EWOULDBLOCK = Errno(_EWOULDBLOCK)' ${OUT} | \ > + sed -i.bak -e 's/_EWOULDBLOCK/_EAGAIN/' ${OUT} I don't understand why you both pass the output of egrep to sed, and you give the -i option to sed. AIUI, the egrep '^const EWOULDBLOCK = Errno(_EWOULDBLOCK)' part is completely unused, so you can just drop it. > @@ -528,6 +540,12 @@ > +# Special treatment of st_dev for GNU/Hurd > +# /usr/include/i386-gnu/bits/stat.h: #define st_dev st_fsid > +if grep 'define st_dev st_fsid' gen-sysinfo.go > /dev/null 2>&1; then > + egrep '^type _stat ' gen-sysinfo.go > /dev/null 2>&1| \ > + sed -i.bak -e 's/; st_fsid/; st_dev/' gen-sysinfo.go > +fi The same remark about egrep | sed -i applies here. And anyway, why not simply using the very first patch you proposed, which was: @@ -536,6 +548,7 @@ fi | sed -e 's/type _stat64/type Stat_t/' \ -e 's/type _stat/type Stat_t/' \ -e 's/st_dev/Dev/' \ + -e 's/st_fsid/Dev/' \ -e 's/st_ino/Ino/g' \ -e 's/st_nlink/Nlink/' \ -e 's/st_mode/Mode/' \ which I said several times that it should be completely fine. Samuel
On Wed, 2014-05-21 at 01:27 +0200, Samuel Thibault wrote: > Svante Signell, le Fri 16 May 2014 10:03:05 +0200, a écrit : > > is used in gcc-4.9-4.9.0/src/libgo/go/net/fd_unix.go: > > func dupCloseOnExec(fd int) (newfd int, err error) { > > if atomic.LoadInt32(&tryDupCloexec) == 1 && syscall.F_DUPFD_CLOEXEC!=0 { > > r0, _, e1 := syscall.Syscall(syscall.SYS_FCNTL, uintptr(fd), > > syscall.F_DUPFD_CLOEXEC, 0) > > That code can not work as it is, fcntl is not a system call on > GNU/Hurd. Why isn't gccgo just using the C fcntl function? That one > will just work and be portable. I don't know, I'm not a go developer. Ask Ian. > > +# Special treatment of EWOULDBLOCK for GNU/Hurd > > +# /usr/include/bits/errno.h: #define EWOULDBLOCK EAGAIN > > +if egrep 'define EWOULDBLOCK EAGAIN' gen-sysinfo.go > /dev/null 2>&1; then > > + egrep '^const EWOULDBLOCK = Errno(_EWOULDBLOCK)' ${OUT} | \ > > + sed -i.bak -e 's/_EWOULDBLOCK/_EAGAIN/' ${OUT} > > I don't understand why you both pass the output of egrep to sed, and you > give the -i option to sed. AIUI, the > egrep '^const EWOULDBLOCK = Errno(_EWOULDBLOCK)' > part is completely unused, so you can just drop it. Well, the -i option is to get a backup copy for debugging purposes, can safely be removed. BTW: On Linux no .bak files are generated, as expected. Regarding the code I attach mksysinfo.sh and config.h.hurd to be run on Hurd and config.h.linux to be run on Linux. Good luck making the patch better. It worked for me, but as I wrote before I'm no sed/grep expert.
Svante Signell, le Wed 21 May 2014 09:47:08 +0200, a écrit : > > > +# Special treatment of EWOULDBLOCK for GNU/Hurd > > > +# /usr/include/bits/errno.h: #define EWOULDBLOCK EAGAIN > > > +if egrep 'define EWOULDBLOCK EAGAIN' gen-sysinfo.go > /dev/null 2>&1; then > > > + egrep '^const EWOULDBLOCK = Errno(_EWOULDBLOCK)' ${OUT} | \ > > > + sed -i.bak -e 's/_EWOULDBLOCK/_EAGAIN/' ${OUT} > > > > I don't understand why you both pass the output of egrep to sed, and you > > give the -i option to sed. AIUI, the > > egrep '^const EWOULDBLOCK = Errno(_EWOULDBLOCK)' > > part is completely unused, so you can just drop it. > > Well, the -i option is to get a backup copy for debugging purposes, can > safely be removed. Err, no, -i completely changes the behavior of sed, which then doesn't read its stdin at all any more, it will modify ${OUT} in-place instead. It happens that it is what you want here. But then just drop the egrep just before, it really is useless now. And drop the .bak suffix, I don't think the gnugo maintainers will want to see a .bak file lying behind. > Good luck making the patch better. It worked for me, but as I wrote > before I'm no sed/grep expert. I'm not the one to be convinced, gnugo maintainers are the one to be. I'm just telling you in advance what *they* will tell you. Samuel
On Wed, May 21, 2014 at 12:47 AM, Svante Signell <svante.signell@gmail.com> wrote: > On Wed, 2014-05-21 at 01:27 +0200, Samuel Thibault wrote: >> Svante Signell, le Fri 16 May 2014 10:03:05 +0200, a écrit : >> > is used in gcc-4.9-4.9.0/src/libgo/go/net/fd_unix.go: >> > func dupCloseOnExec(fd int) (newfd int, err error) { >> > if atomic.LoadInt32(&tryDupCloexec) == 1 && syscall.F_DUPFD_CLOEXEC!=0 { >> > r0, _, e1 := syscall.Syscall(syscall.SYS_FCNTL, uintptr(fd), >> > syscall.F_DUPFD_CLOEXEC, 0) >> >> That code can not work as it is, fcntl is not a system call on >> GNU/Hurd. Why isn't gccgo just using the C fcntl function? That one >> will just work and be portable. > > I don't know, I'm not a go developer. Ask Ian. It's a bug. That code, like most of libgo, is simply copied from the master Go library, and I never noticed the direct use of syscall.Syscall here. Ian
--- a/src/libgo/mksysinfo.sh +++ b/src/libgo/mksysinfo.sh @@ -210,6 +210,13 @@ egrep '#define E[A-Z0-9_]+ ' | \ sed -e 's/^#define \(E[A-Z0-9_]*\) .*$/const \1 = Errno(_\1)/' >> ${OUT} +# Special treatment of EWOULDBLOCK for GNU/Hurd +# /usr/include/bits/errno.h: #define EWOULDBLOCK EAGAIN +if egrep 'define EWOULDBLOCK EAGAIN' gen-sysinfo.go > /dev/null 2>&1; then + egrep '^const EWOULDBLOCK = Errno(_EWOULDBLOCK)' ${OUT} | \ + sed -i.bak -e 's/_EWOULDBLOCK/_EAGAIN/' ${OUT} +fi + # The O_xxx flags. egrep '^const _(O|F|FD)_' gen-sysinfo.go | \ sed -e 's/^\(const \)_\([^= ]*\)\(.*\)$/\1\2 = _\2/' >> ${OUT} @@ -225,6 +232,11 @@ echo "const F_DUPFD_CLOEXEC = 0" >> ${OUT} fi +# Special treatment of SYS_FCNTL for GNU/Hurd +if ! grep '^const SYS_FCNTL' ${OUT} > /dev/null 2>&1; then + echo "const SYS_FCNTL = 0" >> ${OUT} +fi + # These flags can be lost on i386 GNU/Linux when using # -D_FILE_OFFSET_BITS=64, because we see "#define F_SETLK F_SETLK64" # before we see the definition of F_SETLK64. @@ -528,6 +540,12 @@ # The stat type. # Prefer largefile variant if available. +# Special treatment of st_dev for GNU/Hurd +# /usr/include/i386-gnu/bits/stat.h: #define st_dev st_fsid +if grep 'define st_dev st_fsid' gen-sysinfo.go > /dev/null 2>&1; then + egrep '^type _stat ' gen-sysinfo.go > /dev/null 2>&1| \ + sed -i.bak -e 's/; st_fsid/; st_dev/' gen-sysinfo.go +fi stat=`grep '^type _stat64 ' gen-sysinfo.go || true` if test "$stat" != ""; then grep '^type _stat64 ' gen-sysinfo.go
On Wed, 2014-05-07 at 10:18 +0200, Svante Signell wrote: > On Tue, 2014-05-06 at 15:26 +0200, Samuel Thibault wrote: Attached is an updated patch8.diff. Arch specific code to src/libgo/mksysinfo.sh has been added, now other systems are not affected by the patch except the SYS_FCNTL part. For that part of the patch without it the build on GNU/Hurd fails. On the other hand SYS_FCNTL is not defined for e.g. GNU/Linux either. This is used in gcc-4.9-4.9.0/src/libgo/go/net/fd_unix.go: func dupCloseOnExec(fd int) (newfd int, err error) { if atomic.LoadInt32(&tryDupCloexec) == 1 && syscall.F_DUPFD_CLOEXEC!=0 { r0, _, e1 := syscall.Syscall(syscall.SYS_FCNTL, uintptr(fd), syscall.F_DUPFD_CLOEXEC, 0) It is yet unknown how the build succeeds on Linux without the SYS_FCNTL being defined? Maybe any the conditions above are not met.