Re: libtar: FTBFS on hurd-i386

[Samuel Thibault <sthibault@debian.org>]

Svante Signell, le Tue 17 Jan 2012 19:24:40 +0100, a écrit :
> +	if (!allocated) {
> +		allocated = 64;
> +		bname = malloc(allocated);
> +		if (!bname) {
> +			allocated = 0;
> +			return NULL;
> +		}

This is useless:
> +		bname[0] = '\0';

> +	}
>  
>  	/* Empty or NULL string gets treated as "." */
>  	if (path == NULL || *path == '\0') {
> -		(void)strcpy(bname, ".");
> +		strcpy(bname, ".");
>  		return(bname);
>  	}



> @@ -135,25 +136,30 @@
>  		if (ti == NULL)
>  			return -1;
>  		ti->ti_ino = s.st_ino;
> -		snprintf(ti->ti_name, sizeof(ti->ti_name), "%s",
> -			 savename ? savename : realname);
> +		name = savename ? savename : realname; 
> +		if ((ti->ti_name = malloc(strlen(name) + 1)) == NULL)
> +			return -1;
> +		snprintf(ti->ti_name, sizeof(ti->ti_name), "%s", name);
>  		libtar_hash_add(td->td_h, ti);
> +		if (ti->ti_name != NULL)
> +			free(ti->ti_name);

Don't free ti_name, it is needed above in the code, when
libtar_hashptr_data() returns a ti value, and then th_set_link() called.

You have to free it from the tar_close() function (which uses
libtar_hash_free), by introducing a tar_ino_free function to replace the
free parameter passed to libtar_hash_free.

>  	/* check if it's a symlink */
>  	if (TH_ISSYM(t))
>  	{
> -		i = readlink(realname, path, sizeof(path));
> +		if ((path = malloc(s.st_size + 1)) == NULL)
> +			return -1;
> +		i = readlink(realname, path, s.st_size);
>  		if (i == -1)

You need to free path here.

>  			return -1;
> -		if (i >= MAXPATHLEN)
> -			i = MAXPATHLEN - 1;
>  		path[i] = '\0';




>  char *
>  th_get_pathname(TAR *t)
>  {
> -	static char filename[MAXPATHLEN];
> +	static char *filename = NULL;
> +	int len;
>  
>  	if (t->th_buf.gnu_longname)
>  		return t->th_buf.gnu_longname;
>  
>  	if (t->th_buf.prefix[0] != '\0')
>  	{
> -		snprintf(filename, sizeof(filename), "%.155s/%.100s",
> +		len = strlen(t->th_buf.prefix) + 1 + strlen(t->th_buf.name) + 1;
> +		if ((filename = malloc(len) + 1) == NULL)
> +			return NULL;

Same story as dirname/basename, you have to free the previous pointer
held by filename.

> +		snprintf(filename, len, "%.155s/%.100s",
>  			 t->th_buf.prefix, t->th_buf.name);
>  		return filename;
>  	}
>  
> -	snprintf(filename, sizeof(filename), "%.100s", t->th_buf.name);
> +	len = strlen(t->th_buf.name);
> +	if ((filename = malloc(len) + 1) == NULL)
> +		return NULL;
> +	snprintf(filename, len, "%.100s", t->th_buf.name);
>  	return filename;
>  }




> +	if ((lnp->ln_save = malloc(len + 1)) == NULL)
> +	if ((lnp->ln_real = malloc(len + 1)) == NULL)

You also need to care about when to free these.

>  int
>  path_hashfunc(char *key, int numbuckets)
>  {
> -	char buf[MAXPATHLEN];
> +	char *buf, *buf_ptr;
>  	char *p;
> +	int i;
>  
> -	strcpy(buf, key);
> +	buf = strdup(key);
>  	p = basename(buf);
> -
> -	return (((unsigned int)p[0]) % numbuckets);
> +	i = ((unsigned int)p[0]) % numbuckets;
> +	free(buf_ptr);

buf_ptr was never assigned, I guess you meant buf.

Samuel