Re: Patch for e2fsprogs and a question about msync()
Svante Signell, le Tue 22 Nov 2011 15:51:18 +0100, a écrit :
> #ifdef MS_SYNC
> if (tdb->map_ptr) {
> tdb_off_t moffset = offset & ~(tdb->page_size-1);
> if (msync(moffset + (char *)tdb->map_ptr,
> length + (offset - moffset), MS_SYNC) != 0) {
> tdb->ecode = TDB_ERR_IO;
> TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_transaction:
> msync failed - %s\n",
> strerror(errno)));
> return -1;
> }
> }
> #endif
That code should check for ENOSYS and EOPNOTSUPP, as it seems to be fine
with not calling msync at all if MS_SYNC is not available...
> tdb.c:(.text+0x2cc6): warning: warning: msync is not implemented and
> will always fail
> make[1]: Leaving directory
> `.../e2fsprogs-1.42~WIP-2011-11-20.modified/debian/BUILD-STD/e2fsck'
>
> Consequences when this code is executed??
An error, which has never been spotted before, but probably just by
luck.
> 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.000000000 +0100
> +++ e2fsprogs-1.42~WIP-2011-11-20.modified/e2fsck/quota.c 2011-11-22 11:30:13.000000000 +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,7 +38,7 @@
>
> 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);
> }
Missing free for qf_name.
> 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.000000000 +0100
> +++ e2fsprogs-1.42~WIP-2011-11-20.modified/lib/quota/mkquota.c 2011-11-22 12:53:53.000000000 +0100
> @@ -67,14 +67,15 @@
> */
> 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);
Same here.
> 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.000000000 +0100
> +++ e2fsprogs-1.42~WIP-2011-11-20.modified/lib/quota/mkquota.h 2011-11-22 12:53:05.000000000 +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.000000000 +0100
> +++ e2fsprogs-1.42~WIP-2011-11-20.modified/lib/quota/quotaio.c 2011-11-22 11:46:36.000000000 +0100
> @@ -52,11 +52,15 @@
> /**
> * Creates a quota file name for given type and format.
> */
> -const char *quota_get_qf_name(int type, int fmt, char *buf)
> +char *quota_get_qf_name(int type, int fmt)
> {
> - if (!buf)
> + int len;
> + char *buf = NULL;
> +
> + len = strlen(basenames[fmt]) + 1 + strlen(extensions[type]) + 1;
> + if( (buf = malloc(len)) == NULL)
> return NULL;
> - snprintf(buf, PATH_MAX, "%s.%s",
> + snprintf(buf, len, "%s.%s",
> basenames[fmt], extensions[type]);
Looks good.
> return buf;
> @@ -66,14 +70,16 @@
> char *path_buf, size_t path_buf_size)
> {
> struct stat qf_stat;
> - char qf_name[PATH_MAX] = {0};
> + char *qf_name = NULL;
>
> if (!mntpt || !path_buf || !path_buf_size)
> return NULL;
>
> strncpy(path_buf, mntpt, path_buf_size);
> strncat(path_buf, "/", 1);
> - strncat(path_buf, quota_get_qf_name(qtype, fmt, qf_name),
> + if ((qf_name = quota_get_qf_name(qtype, fmt)) == NULL)
> + return NULL;
> + strncat(path_buf, qf_name,
> path_buf_size - strlen(path_buf));
>
> return path_buf;
Missing free here too.
Samuel
Reply to: