On Fri, Jul 13, 2012 at 03:23:26AM +0200, Jan Klemkow wrote:

> +       char *cmd, *tp, *xargv[] = {argv[0], NULL, NULL};
Put spaces inside {}:  { argv[0], NULL, NULL }

> It took me a while to figure out what that code does, so I think the
> comments are usefull for everybody who tries to read it.

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

It is spelled "recursively".

> The file type information comes from this special remglob2() function.
> There is no equivalent thing in the context of mput().  So I check the
> local file type with stat(2) and S_ISDIR().

>                       if (mflag && confirm(argv[0], *cpp)) {
> +
> +                             /*
> +                              * If file is a directory then create a new one
> +                              * at the remote machine.
> +                              */
> +                             stat(*cpp, &filestat);
> +
> +                             if (S_ISDIR(filestat.st_mode)) {
> +
> +                                     if (depth == max_depth)
> +                                             continue;

With maximum depth mput() may prompt for directories it will skip
later.  So move the check before the prompt like in mget():

                        if (!mflag)
                                continue;
                        if (stat(*cpp, &filestat) == -1)
                                continue;
                        if (S_ISDIR(filestat.st_mode) && depth == max_depth)
                                continue;
                        if (!confirm(argv[0], *cpp))
                                continue;
                        if (S_ISDIR(filestat.st_mode)) {

> The makedir() function has no return value, so it is not posible at the
> moment to detect an error inside that function.

> +                                     xargv[1] = *cpp;
> +                                     makedir(2, xargv);
> +
> +                                     /*
> +                                      * Copy the hole directory recursivly.
> +                                      */
> +                                     if (chdir(*cpp) != 0) {
> +                                             warn("local: %s", *cpp);
> +                                             continue;
> +                                     }
> +
> +                                     xargv[1] = *cpp;
> +                                     cd(2, xargv);
> +                                     if (dirchange != 1) {
> +                                             mflag = 0;
> +                                             goto out;
> +                                     }

To verify that makedir() succeeded, do a cd() immediately after it.
Reorder the function calls like this:

                                        /*
                                         * Copy the hole directory recursivly.
                                         */
                                        if (chdir(*cpp) != 0) {
                                                warn("local: %s", *cpp);
                                                continue;
                                        }
   
                                        xargv[1] = *cpp;
                                        makedir(2, xargv);
                                        cd(2, xargv);
                                        if (dirchange != 1) {
                                                mflag = 0;
                                                goto out;
                                        }

bluhm

Reply via email to