Re: e2fsprogs patch for review (updated)
On Wed, 2011-10-05 at 15:34:53 +0200, Svante Signell wrote:
> I created a configure.in check for quota.h and #ifdef'd on that instead
> since building without libquota.a created a lot of undefined references
> when linking.
> See above and the attached updated patch, the number of changes became
> rather many. Guillem, I used your advice to let get_qf_name() return the
> qf_name, instead of having it both in the argument list and as a return
> value. free() does not seem to apply when using the const attribute!?
If the value has to be free()d then it cannot be const, because it has
to modify it.
> Ready to submit as a Debian bug?
It seems the maintainer just uploaded a fixed package today, so some
or most of your changes might not be needed anymore or might just not
apply.
> diff -ur e2fsprogs-1.42~WIP-2011-10-01/lib/quota/mkquota.c e2fsprogs-1.42~WIP-2011-10-01.modified//lib/quota/mkquota.c
> --- e2fsprogs-1.42~WIP-2011-10-01/lib/quota/mkquota.c 2011-09-18 20:06:52.000000000 +0200
> +++ e2fsprogs-1.42~WIP-2011-10-01.modified//lib/quota/mkquota.c 2011-10-05 12:26:13.000000000 +0200
> @@ -48,12 +48,16 @@
>
> int is_quota_on(ext2_filsys fs, int type)
> {
> +#ifdef HAVE_QUOTA_H
> char tmp[1024];
> qid_t id = (type == USRQUOTA) ? getuid() : getgid();
>
> if (!quotactl(QCMD(Q_V2_GETQUOTA, type), fs->device_name, id, tmp))
> return 1;
> return 0;
> +#else
> + return 1;
> +#endif
> }
The value returned is a bool equivalent, so when no quota support, it
should be 0.
> @@ -62,14 +66,15 @@
> */
> int quota_file_exists(ext2_filsys fs, int qtype, int fmt)
> {
> - char qf_name[256];
> + const char *qf_name;
> errcode_t ret;
> ext2_ino_t ino;
>
> if (qtype >= MAXQUOTAS || fmt > QFMT_VFS_V1)
> return -EINVAL;
>
> - get_qf_name(qtype, fmt, qf_name);
> + if ((qf_name = get_qf_name(qtype, fmt)) == NULL)
> + return 0;
Probably better to return -ENOMEM.
> diff -ur e2fsprogs-1.42~WIP-2011-10-01/lib/quota/quotaio.c e2fsprogs-1.42~WIP-2011-10-01.modified//lib/quota/quotaio.c
> --- e2fsprogs-1.42~WIP-2011-10-01/lib/quota/quotaio.c 2011-09-29 00:34:41.000000000 +0200
> +++ e2fsprogs-1.42~WIP-2011-10-01.modified//lib/quota/quotaio.c 2011-10-05 15:14:56.000000000 +0200
> @@ -52,10 +52,15 @@
> /**
> * Creates a quota file name for given type and format.
> */
> -const char *get_qf_name(int type, int fmt, char *buf)
> +const char *get_qf_name(int type, int fmt)
> {
> - BUG_ON(!buf);
> - snprintf(buf, PATH_MAX, "%s.%s",
> + int len;
> + char *buf=NULL;
Small style nit, missing spaces between =.
thanks,
guillem
Reply to: