Comments inline:

On Mon, Jul 09, 2012 at 02:04:27PM +0200, Jan Klemkow wrote:
> Index: cmds.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/ftp/cmds.c,v
> retrieving revision 1.70
> diff -u -p -r1.70 cmds.c
> --- cmds.c    5 May 2009 19:35:30 -0000       1.70
> +++ cmds.c    9 Jul 2012 11:56:54 -0000
> @@ -229,17 +229,30 @@ void
>  mput(int argc, char *argv[])
>  {
>       extern int optind, optreset;
> -     int ch, i, restartit = 0;
> +     int ch, i, restartit = 0, recursiv = 0;
s/recursiv/recursive/
And this variable is not used.

>       sig_t oldintr;
>       char *cmd, *tp;
> -
> +     const char *errstr;
> +     static int depth = 0, max_depth = 0;
Put an emtpy line after the declarations

>       optind = optreset = 1;
>  
> -     while ((ch = getopt(argc, argv, "c")) != -1) {
> +     while ((ch = getopt(argc, argv, "cd:r")) != -1) {
>               switch(ch) {
>               case 'c':
>                       restartit = 1;
>                       break;
> +             case 'd':
> +                     max_depth = strtonum(optarg, 0, INT_MAX, &errstr);
> +                     if (errstr != NULL) {
> +                             fprintf(ttyout, "bad depth value, %s: %s",
\n missing

> +                                 errstr, optarg);
> +                             code = -1;
> +                             return;
> +                     }
> +                     break;
> +             case 'r':
> +                     depth = 1;
> +                     break;
>               default:
>                       goto usage;
>               }
> @@ -247,7 +260,8 @@ mput(int argc, char *argv[])
>  
>       if (argc - optind < 1 && !another(&argc, &argv, "local-files")) {
>  usage:
> -             fprintf(ttyout, "usage: %s [-c] local-files\n", argv[0]);
> +             fprintf(ttyout, "usage: %s [-cr] [-d depth] local-files\n",
> +                 argv[0]);
>               code = -1;
>               return;
>       }
> @@ -318,11 +332,16 @@ usage:
>               mflag = 0;
>               return;
>       }
> +
> +     if (depth)
> +             depth++;
> +
Should this be before getopt like in mget?
After -r depth = 1 case, depth gets incremented to 2 here and is
not reset correctly to 0 after all.  Try mput -r and mput, the
second one will still be recursive.

>       for (i = 1; i < argc; i++) {
>               char **cpp;
>               glob_t gl;
>               int flags;
>  
> +             /* Copy files without word expantion */
s/expantion/expansion/
Needless comments, unecessary diff.

>               if (!doglob) {
>                       if (mflag && confirm(argv[0], argv[i])) {
>                               tp = (ntflag) ? dotrans(argv[i]) : argv[i];
> @@ -348,6 +367,7 @@ usage:
>                       continue;
>               }
>  
> +             /* expanding file names */
Needless comments, unecessary diff.

>               memset(&gl, 0, sizeof(gl));
>               flags = GLOB_BRACE|GLOB_NOCHECK|GLOB_QUOTE|GLOB_TILDE;
>               if (glob(argv[i], flags, NULL, &gl) || gl.gl_pathc == 0) {
> @@ -355,8 +375,58 @@ usage:
>                       globfree(&gl);
>                       continue;
>               }
> +
> +             /* traverse all expanded file names */
Needless comments, unecessary diff.

>               for (cpp = gl.gl_pathv; cpp && *cpp != NULL; cpp++) {
> +                     struct stat filestat;
> +
>                       if (mflag && confirm(argv[0], *cpp)) {
> +
> +                             /* 
trailing whitespace

> +                              * If file is a directory then create a new one
> +                              * at the remote machine.
> +                              */
> +                             stat(*cpp, &filestat);
> +
> +                             if (S_ISDIR(filestat.st_mode)) {
mget() has this logic to skip on directories
                if (type == 'd' && depth == max_depth)
                        continue;
                if (!confirm(argv[0], cp))
                        continue;
                if (type == 'd') {
Perhaps we want the same for mput()

> +                                     char* xargv[] = {argv[0], *cpp};
terminating NULL is missing.
Make it consistent to mget() and declare at function beginning.
Then you get original argv[0], before optind has been added.

> +
> +                                     if (depth == max_depth) {
> +                                             continue;
> +                                     }
no { } for one line if

> +
> +                                     makedir(2, xargv);
These should be right before cd(), as cd() makes the error checking for
makedir().

> +
> +                                     /*
> +                                      * Copy the hole directory recursivly.
> +                                      */
s/recursivly/recursively

> +                                     if (chdir(*cpp) != 0) {
> +                                             warn("local: %s", *cpp);
> +                                             continue;
> +                                     }
> +
> +                                     xargv[1] = *cpp;
> +                                     cd(2, xargv);
> +                                     if (dirchange != 1) {
> +                                             mflag = 0;
> +                                             goto out;
> +                                     }
> +
> +                                     xargv[1] = "*";
> +                                     mput(2, xargv);
> +
> +                                     xargv[1] = "..";
> +                                     cd(2, xargv);
> +                                     if (dirchange != 1) {
> +                                             mflag = 0;
Place a goto out here like in mget() to be consistent.  Or remove the { }.

> +                                     }
> + out:
> +                                     if (chdir("..") != 0) {
> +                                             warn("local: %s", *cpp);
mget() does a mflag = 0 here.  Why not for mput?

> +                                     }
> +                                     continue;
> +                             }
> +
>                               tp = (ntflag) ? dotrans(*cpp) : *cpp;
>                               tp = (mapflag) ? domap(tp) : tp;
>                               if (restartit == 1) {
> @@ -380,8 +450,14 @@ usage:
>               }
>               globfree(&gl);
>       }
> +
>       (void)signal(SIGINT, oldintr);
> -     mflag = 0;
> +
> +     if (depth)
> +             depth--;
> +
> +     if (depth == 0 || mflag == 0)
> +             depth = max_depth = mflag = 0;
use the same spacing as in mget() i.e. remove empty lines

>  }
>  
>  void
> Index: ftp.1
> ===================================================================
> RCS file: /cvs/src/usr.bin/ftp/ftp.1,v
> retrieving revision 1.82
> diff -u -p -r1.82 ftp.1
> --- ftp.1     30 Apr 2012 13:41:26 -0000      1.82
> +++ ftp.1     9 Jul 2012 11:56:54 -0000
> @@ -664,7 +664,8 @@ on the remote machine.
>  A synonym for
>  .Ic page .
>  .It Xo Ic mput
> -.Op Fl c
> +.Op Fl cr
> +.Op Fl d Ar depth
>  .Ar local-files
>  .Xc
>  Expand wild cards in the list of local files given as arguments
> @@ -683,9 +684,21 @@ settings.
>  If the
>  .Fl c
>  flag is specified then
> +The options are as follows:
> +.Bl -tag -width Ds
> +.It Fl c
> +Use
>  .Ic reput
> -is used instead of
> +instead of
>  .Ic put .
> +.It Fl d Ar depth
> +Specify the maximum recursion level
> +.Ar depth .
> +The default is 0, which means unlimited.
> +.It Fl r
> +Recursively descend the directory tree, transferring all files and
> +directories.
> +.El
>  .It Xo Ic msend
>  .Op Fl c
>  .Ar local-files

bluhm

Reply via email to