Rolf,
Thanks for cleaning up SHFileOperation.
Can you please try restructuring your code further to get rid of the
excessive number of branching points in that function? Last time I
counted, SHFileOperation had more than 60 if/while/for/case statements
in the one +500 line function... that makes it hard to read.
If you were to make use of helper functions more often, you'd improve
your code's readability, eliminate the need for 2-space indent, the
pointless /* end-if */ comments, reduce the excessive number of local
variables and reduce side effects, thus making it easier for other
people to understand and fix the code.
Try make small functions that fit in ~50 lines. If a function gets over
50 lines, cut it up into two new functions.
Mike
Rolf Kalbermatter wrote:
> + if (b_Multi || IsAttribFile(FromAttr))
> + continue;
> + if (IsAttribDir(FromAttr))
> + {
> + /* in w98/w2k/wXp endless recursive move until it aborts due to filename getting to
> + long, we don't do this stupidity but return the according error anyhow here */
> + retCode = ERROR_FILENAME_EXCED_RANGE;
> + break;
> + }
> }
> - }
> + if (( b_SameRoot_MOVE && (b_Multi) && !(b_MultiFrom) && IsAttribFile(FromAttr) && IsAttribDir(ToAttr))
> + || (!(b_MultiFrom) && IsAttribFile(FromAttr) && IsAttribFile(ToAttr))
> + || ( (b_Multi) && IsAttribDir(FromAttr) && IsAttribDir(ToAttr))
> + || (!b_SameRoot_MOVE && IsAttribDir(FromAttr) && IsAttribFile(ToAttr))
> + )
> + {
> + ToAttr = SHRenameOnCollision(&nFileOp, pToFile + 1, pFromFile + 1);
> + } /* endif */
> + } /* endif */
> + if (!(b_MultiFrom) && (!FlagRenameOnCollision(&nFileOp) || b_SameRoot_MOVE) &&
> + ((SFSC_IDENTIICAL | SFSC_SOURCE_CONTAINS_TARGET) & f_SameDrive))
> + {
> + if (!FlagRenameOnCollision(&nFileOp) && (IsAttribFile(FromAttr)))
> + {
> + /* target is the same as source */
> + retCode = 0x71;
> + }
> + break;
> + } /* endif */
> +
> + /* Analyzing for moving SourceName to TargetName */
> + if ((b_MultiFrom || !b_Multi) && ((IsAttribFile(FromAttr) && IsAttribDir(ToAttr)) ||
> + (!b_MultiTo && IsAttribDir(ToAttr)) ||
> + (!b_MultiTo && IsAttribDir(FromAttr) && b_MultiFrom && !b_SameRoot_MOVE)))
> + {
> + SHFileStrCpyCatW(pTempTo, NULL, pFromFile);
> + /* without FOF_MULTIDESTFILES shlfileop would create dir's recursively */
> + nFileOp.fFlags |= FOF_MULTIDESTFILES;
> + retCode = SHFileOperationW(&nFileOp);
> + continue;
> + }
> + /* What can we do with one pair and FO_MOVE/FO_COPY ? */
> + /* w98se, nt40 does create recursive dirs, W2K not! wMe ist not tested */
> + if (IsAttribDir(ToPathAttr) || !b_SameRoot_MOVE)
> + {
> + if (!b_MultiFrom)
> + {
> + if (IsAttribFile(FromAttr))
> + {
> + if (b_SameRoot_MOVE &&
> + /* windows-bug, MOVE for File also with pToTailSlash, COPY not for this */
> + (!(IsAttribFile(ToAttr)) || (!(FlagAskOverwrite(&nFileOp)) && FlagNotOverwrite(&nFileOp))) &&
> + (IsAttribDir(ToPathAttr) || b_ToMask))
> + {
> + /* With the same drive, we can move for FO_MOVE, dir to dir is solved later */
> + retCode = SHNotifyMoveFileW(nFileOp.pFrom, nFileOp.pTo);
> + continue;
> + } /* endif */
> + if (IsAttribDir(ToPathAttr) && !pToTailSlash && !IsAttribDir(ToAttr))
> + {
> + if (!(FOI_NeverOverwrite & nFileOp.fAnyOperationsAborted))
> + {
> + if (!FlagNotOverwrite(&nFileOp))
> + {
> + ToAttr = INVALID_FILE_ATTRIBUTES;
> + }
> + if (IsAttribFile(ToAttr) && FlagNotOverwrite(&nFileOp))
> + {
> + if (FlagAskOverwrite(&nFileOp))
> + {
> + /* FIXME: we must change the dialog in the future to allow 3-4 cases,
> + * 'Yes','No','Yes for all','Never ?' */
> + if ((INVALID_FILE_ATTRIBUTES != ToAttr) && !SHELL_ConfirmDialogW(ASK_OVERWRITE_FILE, pTempTo))
> + {
> + retCode = 0x10008;
> + nFileOp.fAnyOperationsAborted |= TRUE;
> + break;
> + }
> + ToAttr = INVALID_FILE_ATTRIBUTES;
> + } /* endif */
> + } /* endif */
> + } /* endif */
> + if (INVALID_FILE_ATTRIBUTES == ToAttr)
> + {
> + if (SHNotifyCopyFileW(pTempFrom, pTempTo, FlagRenameOnCollision(&nFileOp)))
> + {
> + /* the retcode for this case is unknown */
> + retCode = 0x10009;
> + }
> + if ((FO_COPY != FuncSwitch) && !retCode)
> + {
> + nFileOp.wFunc = ((level + 1) << FO_LevelShift) + FO_DELETE;
> + retCode = SHFileOperationW(&nFileOp);
> + }
> + continue;
> + }
> + }
> + }
> + if (IsAttribDir(FromAttr))
> + {
> + if (INVALID_FILE_ATTRIBUTES == ToAttr)
> + {
> + SetIfPointer(pToTailSlash, '\0');
> + ToAttr = SHRenameOnCollision(&nFileOp, pToFile + 1, pFromFile + 1);
> + if (INVALID_FILE_ATTRIBUTES != ToAttr)
> + {
> + /* w2k returns ERROR_ALREADY_EXISTS, if it found the target with mask */
> +#if 1
> + retCode = ERROR_ALREADY_EXISTS;
> +#else
> + retCode = ERROR_INVALID_NAME;
> +#endif
> + nFileOp.fAnyOperationsAborted |= TRUE;
> + break;
> + }
> + lpFileName = &pToFile[lstrlenW(pToFile)];
> + if (pToTailSlash)
> + {
> + pToTailSlash = lpFileName;
> + lpFileName++;
> + }
> + ((DWORD*)lpFileName)[0] = '\0';
> + retCode = SHNotifyCreateDirectoryW(pTempTo, NULL);
> +
> + if (retCode)
> + {
> + retCode = (ERROR_PATH_NOT_FOUND | 0x10000); /* nt40,w98se,w2k,wxp */
> + break;
> + }
> + ToAttr = GetFileAttributesW(pTempTo);
> + SetIfPointer(pToTailSlash,'\\');
> + nFileOp.fFlags &= ~FOF_RENAMEONCOLLISION;
> + }
> + if (IsAttribDir(ToAttr))
> + {
> + /* both are existing dirs, we (FO_)MOVE/COPY the content */
> + pToFile = SHFileStrCpyCatW(pTempFrom, NULL, wWildcardFile);
> + nFileOp.fFlags &= ~FOF_MULTIDESTFILES;
> + retCode = SHFileOperationW(&nFileOp);
> + if ((FO_COPY != FuncSwitch) && !retCode)
> + {
> + ((DWORD*)pToFile)[0] = '\0';
> + nFileOp.wFunc = ((level + 1) << FO_LevelShift) + FO_DELETE;
> + retCode = SHFileOperationW(&nFileOp);
> + }
> + continue;
> + }
> + } /* endif IsAttribDir(from) */
> + } /* endif !b_MultiFrom */
> + }
> + if (!b_SameRoot_MOVE)
> + {
> + nFileOp.fAnyOperationsAborted |= TRUE;
> + } /* endif */
> + retCode = ERROR_CANCELLED;
> + } /* end-while */