Re: e2fsprogs patch for review (and other stuff)

[Samuel Thibault <sthibault@debian.org>]

Svante Signell, le Tue 04 Oct 2011 17:53:49 +0200, a écrit :
> 1) It seems that more and more code becoming is Linux specific. People
> don't seem to care about portability :-(
> 2) Many Debian maintainers don't seem to care about other arches like
> Hurd/kFreeBSD either. Is there something that can be made to improve
> this situation.

They actually *have* to care about kFreeBSD, since it's a released arch.

> 3) Maybe more frequent releases of gnumach/hurd/eglibc could be created.
> Maybe using experimental would be a good idea?

Lack of manpower.

> Even if Samuel creates the build summaries like summary.txt and
> graph-total-top.txt there is no priority list on which packages are most
> important to concentrate on. Such a list would be very useful for people
> working with package porting.

Lack of manpower.

> +#ifdef __linux__
>  static struct str_table generic_code_table[] = {
>  	DEFINE_ENTRY(SI_ASYNCNL)
>  	DEFINE_ENTRY(SI_TKILL)
> @@ -143,6 +144,16 @@
>  	DEFINE_ENTRY(SI_KERNEL)
>  	END_TABLE
>  };
> +#else
> +static struct str_table generic_code_table[] = {
> +	DEFINE_ENTRY(SI_ASYNCIO)
> +	DEFINE_ENTRY(SI_MESGQ)
> +	DEFINE_ENTRY(SI_TIMER)
> +	DEFINE_ENTRY(SI_QUEUE)
> +	DEFINE_ENTRY(SI_USER)
> +	END_TABLE
> +};
> +#endif

Maybe rather use #ifdef SI_TKILL around the SI_TKILL entry, etc.?

> --- 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-04 15:25:26.000000000 +0200
> @@ -51,9 +51,13 @@
>  	char tmp[1024];
>  	qid_t id = (type == USRQUOTA) ? getuid() : getgid();
>  
> +#ifndef __GNU__
>  	if (!quotactl(QCMD(Q_V2_GETQUOTA, type), fs->device_name, id, tmp))
>  		return 1;
>  	return 0;
> +#else

Maybe rather use #ifdef Q_V2_GETQUOA?

> @@ -55,7 +56,11 @@
>  const char *get_qf_name(int type, int fmt, char *buf)
>  {
>  	BUG_ON(!buf);
> -	snprintf(buf, PATH_MAX, "%s.%s",
> +	int len;
> +	len = strlen(basenames[fmt]) + 1 + strlen(extensions[type]) + 1;
> +	if( (buf = malloc(len)) == NULL)
> +		 assert(buf);

Take care of such situation: assigning buf = malloc(len) will *not*
change the value of the pointer in the caller, that is

> @@ -65,7 +70,7 @@
>  			char *path_buf, size_t path_buf_size)
>  {
>  	struct stat	qf_stat;
> -	char qf_name[PATH_MAX] = {0};
> +	char *qf_name = NULL;

This one. You have to pass the address of the pointer to permit
get_qf_name to change it. You'll also have to take care about
allocations/frees to not leak.

Samuel