Date: Sat, 17 Feb 2001 09:45:41 +0200
From: "Eli Zaretskii" <eliz AT is DOT elta DOT co DOT il>
Sender: halo1 AT zahav DOT net DOT il
To: lauras AT softhome DOT net
Message-Id: <2110-Sat17Feb2001094540+0200-eliz@is.elta.co.il>
X-Mailer: Emacs 20.6 (via feedmail 8.3.emacs20_6 I) and Blat ver 1.8.6
CC: djgpp-workers AT delorie DOT com
In-reply-to: <20010216102653.32267.qmail@softhome.net> (lauras@softhome.net)
Subject: Re: djtar: skipping directories
References:  <20010216102653 DOT 32267 DOT qmail AT softhome DOT net>
Reply-To: djgpp-workers AT delorie DOT com
Errors-To: nobody AT delorie DOT com
X-Mailing-List: djgpp-workers AT delorie DOT com
X-Unsubscribes-To: listserv AT delorie DOT com
Precedence: bulk

> From: lauras AT softhome DOT net
> Date: Fri, 16 Feb 2001 10:26:53 GMT
> 
> Here is an updated patch, which does:

Thanks.

> Criticism is welcome, as always :)

Here it goes:

> +@pindex djtar AT r{, new option -e}
> +@pindex djtar AT r{, multiple -o and -e options}
> +New option @samp{-e} has been added to @code{djtar} to skip specified
> files 
> +and directories when extracting. Also @code{djtar} can accept multiple 
> +@samp{-o} and @samp{-e} options in a single command line.  Finally, the
> file
> +name change file format allows skipping files and directories.

One-space-after-period strikes again ;-)

>  The directories must be complete, not relative.  The "old" directories
>  must match the complete path in the tar file, and the "new" directories
> -indicate where the file goes on the DOS disk.
> +indicate where the file goes on the DOS disk.  If there is no "new"
> directory
> +specified, the "old" one and all its siblings will be not extracted.

Texinfo sources should use `` and '', not " and ", to quote text.
(Yes, this is a bug in the original version.)

> +wildcard facility.  You may specify multiple @samp{-o} options to extract
> +few different directories and files.

I think ``several'' is better here than ``few''.

> +static void *
> +djtmalloc(const size_t size)
> +{
> +   void * ptr = malloc(size);
> +   if (!ptr)
> +      Fatal("Out of memory");
> +   return ptr;

Shouldn't you simply use xmalloc?

> +  if (*should_be_written)
> +  {
> +     skip_dir_entry = skip_dirs;
> +     while (skip_dir_entry)
> +     {
> +        if (!strncmp(skip_dir_entry->skip_dir, name_to_change, 
> +                     strlen(skip_dir_entry->skip_dir)))
> +           break;
> +        skip_dir_entry = skip_dir_entry->next;
> +     }
> +     if (skip_dir_entry)
> +        *should_be_written = 0;

This doesn't seem right.  Suppose you have a line in the name-change
file which says this:

     /foo/bar/baz

That is, you want to skip that file or directory.  Given this line,
the above code would skip all files whose names begin with
"/foo/bar/baz", including, for example, "/foo/bar/bazilicum.c" and
"/foo/bar/bazillion/xyzzy".

I think you need to check that the next character after the ones you
compare is either a slash '/' or a null '\0'.

> +      case 'e':
> +        skip_entry = djtmalloc(sizeof(struct skip_dir_list));
> +        skip_entry->skip_dir = strdup(argv[++i]);

If we are cautious about NULL pointers being returned by malloc, why
not do the same for strdup?