Bug#649689: e2fsprogs: FTBFS on hurd-i386
On Mon, 2011-11-28 at 15:32 -0500, Ted Ts'o wrote:
> On Mon, Nov 28, 2011 at 07:37:53PM +0100, Svante Signell wrote:
> >
> > Checking for HAVE_MSYNC helps because configure finds the stub and
> > decides it is not usable, see below.
>
> Um, how does configure find the stub if you're just doing an
>
> AC_CHECK_FUNCS(... msync ...)
>
> That just checks if a function can link against msync() and not fail.
> It's not going to cause configure to go peering deeply into header
> files. Are you proposing that we use some specific autoconf macro?
> If so, which macro? Your patch just uses AC_CHECK_FUNCS, which
> doesn't have any kind of hueristic magic or artificial intelligence
> capabilities as far as I know
Yes, it came as a surprise for me too:
On GNU/Linux
>From the build log:
checking for msync... yes
>From ./debian/BUILD-STD/config.log:
configure:10910: checking for msync
configure:10910: gcc -o conftest -D__NO_STRING_INLINES conftest.c
-lblkid >&5
configure:10910: $? = 0
configure:10910: result: yes
ON GNU/Hurd
===
>From the build log:
checking for msync... no
>From ./debian/BUILD-STD/config.log:
configure:10910: checking for msync
configure:10910: gcc -o conftest -D__NO_STRING_INLINES conftest.c
-lblkid >&5
conftest.c:196:1: error: unknown type name 'choke'
conftest.c:199:1: error: expected '=', ',', ';', 'asm' or
'__attribute__' before 'int'
configure:10910: $? = 1
configure: failed program was:
| /* confdefs.h */
...
| #define HAVE_BACKTRACE 1
| /* end confdefs.h. */
| /* Define msync to an innocuous variant, in case declares
msync.
|For example, HP-UX 11i declares gettimeofday. */
| #define msync innocuous_msync
|
| /* System header to define __stub macros and hopefully few prototypes,
| which can conflict with char msync (); below.
| Prefer to if __STDC__ is defined, since
| exists even on freestanding compilers. */
|
| #ifdef __STDC__
| # include
| #else
| # include
| #endif
|
| #undef msync
|
| /* Override any GCC internal prototype to avoid an error.
|Use char because int might match the return type of a GCC
|builtin and then its argument prototype would still apply. */
| #ifdef __cplusplus
| extern "C"
| #endif
| char msync ();
| /* The GNU C library defines this for functions which it implements
| to always fail with ENOSYS. Some functions are actually named
| something starting with __ and the normal name is an alias. */
| #if defined __stub_msync || defined __stub___msync
| choke me
| #endif
|
| int
| main ()
| {
| return msync ();
| ;
| return 0;
| }
configure:10910: result: no
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#649689: e2fsprogs: FTBFS on hurd-i386
On Mon, Nov 28, 2011 at 07:37:53PM +0100, Svante Signell wrote: > > Checking for HAVE_MSYNC helps because configure finds the stub and > decides it is not usable, see below. Um, how does configure find the stub if you're just doing an AC_CHECK_FUNCS(... msync ...) That just checks if a function can link against msync() and not fail. It's not going to cause configure to go peering deeply into header files. Are you proposing that we use some specific autoconf macro? If so, which macro? Your patch just uses AC_CHECK_FUNCS, which doesn't have any kind of hueristic magic or artificial intelligence capabilities as far as I know - Ted -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#649689: e2fsprogs: FTBFS on hurd-i386
On Mon, 2011-11-28 at 12:33 -0500, Ted Ts'o wrote:
> On Sun, Nov 27, 2011 at 10:27:48PM -0500, Ted Ts'o wrote:
> > > Additionally, the call to msync() is made conditional on HAVE_MSYNC
> > > instead of MS_SYNC by a test in configure.in for that function. On
> > > GNU/Hurd msync() is only a stub, so with this test a potential run-time
> > > error is avoided.
>
> Wait a second --- do you mean that msync() doesn't exist, but it
> exists but is a no-op?
MC_SYNC is defined in /usr/include/i386-gnu/bits/mman.h on GNU/Hurd, so
the test for that succeeds.
> If it's the latter ("is only a stub"), then adding a check for
> HAVE_MSYNC won't help, because configure will check for the stub, find
> it, and come to the wrong conclusion.
Checking for HAVE_MSYNC helps because configure finds the stub and
decides it is not usable, see below.
.../eglibc-2.13/sysdeps/mach/msync.c:
/* Some Mach variants have vm_msync and some don't. Those that have it
define the VM_SYNC_* bits when we include . */
#ifndef VM_SYNC_SYNCHRONOUS
# include
#else
...
#endif
.../eglibc-2.13/misc/msync.c
int
msync (__ptr_t addr, size_t len, int flags)
{
__set_errno (ENOSYS);
return -1;
}
stub_warning (msync)
#include
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#649689: e2fsprogs: FTBFS on hurd-i386
On Sun, Nov 27, 2011 at 10:27:48PM -0500, Ted Ts'o wrote:
> > Additionally, the call to msync() is made conditional on HAVE_MSYNC
> > instead of MS_SYNC by a test in configure.in for that function. On
> > GNU/Hurd msync() is only a stub, so with this test a potential run-time
> > error is avoided.
Wait a second --- do you mean that msync() doesn't exist, but it
exists but is a no-op?
If it's the latter ("is only a stub"), then adding a check for
HAVE_MSYNC won't help, because configure will check for the stub, find
it, and come to the wrong conclusion.
- Ted
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#649689: e2fsprogs: FTBFS on hurd-i386
tags 649689 +pending -patch
thanks
On Wed, Nov 23, 2011 at 09:44:34AM +0100, Svante Signell wrote:
> The attached patch fixes the FTBFS problems for GNU/Hurd due to the lack
> of a PATH_MAX definition. Instead of fixed length strings dynamic
> allocation is used.
I fixed this problem a different way. See below.
> Additionally, the call to msync() is made conditional on HAVE_MSYNC
> instead of MS_SYNC by a test in configure.in for that function. On
> GNU/Hurd msync() is only a stub, so with this test a potential run-time
> error is avoided.
I'll take care of this too --- but in the future I'd appreciate if you
submitted separate patches for separate fixes.
Thanks, regards
- Ted
commit 4cd2d4ca4128456b82e927b1efd2dd023100934e
Author: Theodore Ts'o
Date: Sun Nov 27 20:31:36 2011 -0500
libquota: remove use of PATH_MAX and replace it with QUOTA_NAME_LEN
PATH_MAX is not portable (for example, it doesn't exist on the Hurd).
So replace it with a new define, which defines the maximum length of
the base quota name. As it turns out, this is substantially smaller
than PATH_MAX.
Also move the definitions relating to quotaio.c from mkquota.h to
quotaio.h, as a cleanup.
Addresses-Debian-Bug: #649689
Signed-off-by: "Theodore Ts'o"
diff --git a/lib/quota/mkquota.h b/lib/quota/mkquota.h
index 4fbaedd..a5fa74b 100644
--- a/lib/quota/mkquota.h
+++ b/lib/quota/mkquota.h
@@ -60,9 +60,4 @@ int quota_is_on(ext2_filsys fs, int type);
int quota_file_exists(ext2_filsys fs, int qtype, int fmt);
void quota_set_sb_inum(ext2_filsys fs, ext2_ino_t ino, int qtype);
-/* In quotaio.c */
-const char *quota_get_qf_name(int type, int fmt, char *buf);
-const char *quota_get_qf_path(const char *mntpt, int qtype, int fmt,
- char *path_buf, size_t path_buf_size);
-
#endif /* __QUOTA_QUOTAIO_H__ */
diff --git a/lib/quota/quotaio.c b/lib/quota/quotaio.c
index d0b5b9d..3f434ca 100644
--- a/lib/quota/quotaio.c
+++ b/lib/quota/quotaio.c
@@ -56,7 +56,7 @@ const char *quota_get_qf_name(int type, int fmt, char *buf)
{
if (!buf)
return NULL;
- snprintf(buf, PATH_MAX, "%s.%s",
+ snprintf(buf, QUOTA_NAME_LEN, "%s.%s",
basenames[fmt], extensions[type]);
return buf;
@@ -66,7 +66,7 @@ const char *quota_get_qf_path(const char *mntpt, int qtype,
int fmt,
char *path_buf, size_t path_buf_size)
{
struct stat qf_stat;
- char qf_name[PATH_MAX] = {0};
+ char qf_name[QUOTA_NAME_LEN];
if (!mntpt || !path_buf || !path_buf_size)
return NULL;
diff --git a/lib/quota/quotaio.h b/lib/quota/quotaio.h
index f8cc1f1..91a1ff2 100644
--- a/lib/quota/quotaio.h
+++ b/lib/quota/quotaio.h
@@ -159,4 +159,12 @@ const char *type2name(int type);
void update_grace_times(struct dquot *q);
+/* size for the buffer returned by quota_get_qf_name(); must be greater
+ than maxlen of extensions[] and fmtnames[] (plus 2) found in quotaio.c */
+#define QUOTA_NAME_LEN 16
+
+const char *quota_get_qf_name(int type, int fmt, char *buf);
+const char *quota_get_qf_path(const char *mntpt, int qtype, int fmt,
+ char *path_buf, size_t path_buf_size);
+
#endif /* GUARD_QUOTAIO_H */
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#649689: e2fsprogs: FTBFS on hurd-i386
Package: e2fsprogs
Version: 1.42~WIP-2011-11-20-1
Severity: important
Tags: patch
User: debian-h...@lists.debian.org
Usertags: hurd
Hello,
The attached patch fixes the FTBFS problems for GNU/Hurd due to the lack
of a PATH_MAX definition. Instead of fixed length strings dynamic
allocation is used.
Additionally, the call to msync() is made conditional on HAVE_MSYNC
instead of MS_SYNC by a test in configure.in for that function. On
GNU/Hurd msync() is only a stub, so with this test a potential run-time
error is avoided.
Thanks!
diff -ur e2fsprogs-1.42~WIP-2011-11-20/configure.in e2fsprogs-1.42~WIP-2011-11-20.modified/configure.in
--- e2fsprogs-1.42~WIP-2011-11-20/configure.in 2011-11-12 03:28:46.0 +0100
+++ e2fsprogs-1.42~WIP-2011-11-20.modified/configure.in 2011-11-23 08:27:58.0 +0100
@@ -914,7 +914,7 @@
AC_SEARCH_LIBS([blkid_probe_all], [blkid])
fi
dnl
-AC_CHECK_FUNCS(chflags getrusage llseek lseek64 open64 fstat64 ftruncate64 getmntinfo strtoull strcasecmp srandom jrand48 fchown mallinfo fdatasync quotactl strnlen strptime strdup sysconf pathconf posix_memalign memalign valloc __secure_getenv prctl mmap utime setresuid setresgid usleep nanosleep getdtablesize getrlimit sync_file_range posix_fadvise fallocate fallocate64 blkid_probe_get_topology mbstowcs backtrace)
+AC_CHECK_FUNCS(chflags getrusage llseek lseek64 open64 fstat64 ftruncate64 getmntinfo strtoull strcasecmp srandom jrand48 fchown mallinfo fdatasync quotactl strnlen strptime strdup sysconf pathconf posix_memalign memalign valloc __secure_getenv prctl mmap utime setresuid setresgid usleep nanosleep getdtablesize getrlimit sync_file_range posix_fadvise fallocate fallocate64 blkid_probe_get_topology mbstowcs backtrace msync)
dnl
dnl Check to see if -lsocket is required (solaris) to make something
dnl that uses socket() to compile; this is needed for the UUID library
diff -ur e2fsprogs-1.42~WIP-2011-11-20/e2fsck/quota.c e2fsprogs-1.42~WIP-2011-11-20.modified/e2fsck/quota.c
--- e2fsprogs-1.42~WIP-2011-11-20/e2fsck/quota.c 2011-11-14 16:55:54.0 +0100
+++ e2fsprogs-1.42~WIP-2011-11-20.modified/e2fsck/quota.c 2011-11-23 08:50:43.0 +0100
@@ -24,7 +24,7 @@
ext2_ino_t ino;
struct ext2_inode inode;
errcode_t retval;
- char qf_name[255];
+ char *qf_name;
if (ext2fs_read_inode(fs, from_ino, &inode))
return;
@@ -38,9 +38,10 @@
ext2fs_write_new_inode(fs, to_ino, &inode);
/* unlink the old inode */
- quota_get_qf_name(qtype, QFMT_VFS_V1, qf_name);
+ qf_name = quota_get_qf_name(qtype, QFMT_VFS_V1);
ext2fs_unlink(fs, EXT2_ROOT_INO, qf_name, from_ino, 0);
ext2fs_inode_alloc_stats(fs, from_ino, -1);
+ free(qf_name);
}
void e2fsck_hide_quota(e2fsck_t ctx)
diff -ur e2fsprogs-1.42~WIP-2011-11-20/lib/ext2fs/tdb.c e2fsprogs-1.42~WIP-2011-11-20.modified/lib/ext2fs/tdb.c
--- e2fsprogs-1.42~WIP-2011-11-20/lib/ext2fs/tdb.c 2011-11-05 19:54:22.0 +0100
+++ e2fsprogs-1.42~WIP-2011-11-20.modified/lib/ext2fs/tdb.c 2011-11-23 08:44:38.0 +0100
@@ -1752,7 +1752,7 @@
TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_transaction: fsync failed\n"));
return -1;
}
-#ifdef MS_SYNC
+#ifdef HAVE_MSYNC
if (tdb->map_ptr) {
tdb_off_t moffset = offset & ~(tdb->page_size-1);
if (msync(moffset + (char *)tdb->map_ptr,
diff -ur e2fsprogs-1.42~WIP-2011-11-20/lib/quota/mkquota.c e2fsprogs-1.42~WIP-2011-11-20.modified/lib/quota/mkquota.c
--- e2fsprogs-1.42~WIP-2011-11-20/lib/quota/mkquota.c 2011-11-14 17:36:12.0 +0100
+++ e2fsprogs-1.42~WIP-2011-11-20.modified/lib/quota/mkquota.c 2011-11-23 08:50:57.0 +0100
@@ -67,17 +67,19 @@
*/
int quota_file_exists(ext2_filsys fs, int qtype, int fmt)
{
- char qf_name[256];
+ char *qf_name;
errcode_t ret;
ext2_ino_t ino;
if (qtype >= MAXQUOTAS)
return -EINVAL;
- quota_get_qf_name(qtype, fmt, qf_name);
+ if ((qf_name = quota_get_qf_name(qtype, fmt)) == NULL)
+ return 0;
ret = ext2fs_lookup(fs, EXT2_ROOT_INO, qf_name, strlen(qf_name), 0,
&ino);
+ free(qf_name);
if (ret)
return 0;
diff -ur e2fsprogs-1.42~WIP-2011-11-20/lib/quota/mkquota.h e2fsprogs-1.42~WIP-2011-11-20.modified/lib/quota/mkquota.h
--- e2fsprogs-1.42~WIP-2011-11-20/lib/quota/mkquota.h 2011-11-14 16:58:28.0 +0100
+++ e2fsprogs-1.42~WIP-2011-11-20.modified/lib/quota/mkquota.h 2011-11-22 12:53:05.0 +0100
@@ -61,7 +61,7 @@
void quota_set_sb_inum(ext2_filsys fs, ext2_ino_t ino, int qtype);
/* In quotaio.c */
-const char *quota_get_qf_name(int type, int fmt, char *buf);
+char *quota_get_qf_name(int type, int fmt);
const char *quota_get_qf_path(const char *mntpt, int qtype, int fmt,
char *path_buf, size_t path_buf_size);
diff -ur e2fsprogs-1.42~WIP-2011-11-20/lib/quota/quotaio.c e2fsprogs-1.42~WIP-2011-11-20.modified/lib/quota/quotaio.c
--- e2fsprogs-1.42~WIP-2011-11-20/lib/quota/quotaio.c 2011-11-14 17:37:26.0 +0100
+++ e2fsprogs-1.42~WIP-2011-11-20.modified/lib/quota/quotaio.c 2011-11-23 08:52:25.0 +0100
@@ -52,1
