Hi! 23-Фев-2004 09:03 [EMAIL PROTECTED] (Kenneth J. Davis) wrote to [EMAIL PROTECTED]:
>>> else if (memicmp(argp, "BOTH", 4) == 0 && !both) >>- AIU, there should be stricmp() (or third parameter of memicmp() should be >> increased to include zero byte)? KJD> Could, but its more of, are exact arguments here really necessary? I think, yes. KJD> There is next to no chance ? KJD> of a user accidently typing the initial part and expecting it to fail KJD> because they [purposely or accidently] tacted on some additional letters. ? Not very understand these sentences. Also, imagine next command line: SYS A: BOOTONLY/K mykernel.sys KJD> Wasn't this originally a stricmp? Don't know. >> printf("%s: drive %c must be A:..Z:\n", pgm, *argv[drivearg]); >> but I think, this diagnostic may be ommited completely. KJD> It can help pinpoint problems (such as what argument one types that it KJD> thinks is the drive This bug will be diagnosted later, by code, which will try to access drive. >> strncpy(srcPath, argv[srcarg], sizeof srcPath - 12); >> /* leave room for COMMAND.COM\0 */ >> srcPath[sizeof srcPath - 12] = '\0'; >>-----------------------------^^ KJD> There are 13 characters in '\\COMMAND.COM\0', which there must be room KJD> for, so the 13 is correct, but the comment could be updated to include KJD> the separator. Precisely. Anyway, in my current code I rewrote this part. >>> if ((srcPath[slen - 1] != ':') && >>> ((srcPath[slen - 1] != '\\') || (srcPath[slen - 1] != '/'))) >>- bug? AIU, should be: >> if (slen) { KJD> I think at some point the preconditions meant this check was not KJD> required, but perhaps I'm mistaken and it was overlooked. Probably, there _was_ ready precondition, but not in code, which I see. >> if (ch != ':' && ch != '\\' && ch != '/') >>-----------------------------------^^ KJD> good catch, I think I originally meant [but screwed up] BTW, who currently officialy mantain SYS? >> srcPath [slen] = '\\', srcPath [slen + 1] = '\0'; KJD> I'd rather see KJD> { KJD> srcPath[slen] = '\\'; KJD> srcPath[slen+1] = '\0'; KJD> } KJD> over the use of the comma operator, as it makes it more clear that a KJD> compound statement is executed. :) Question of taste, which easy to fix/change. KJD> slen++ or [slen+1] makes little difference to me KJD> (whichever produces more efficent code) This depends. In case if srcPath is an array, then additional constant offset is "hidden" in other displacements. If srcPath is a pointer, you simply get one-byte instruction increase, but this removes _extra_ instruction(s) to increase slen (which later not used). >>> /* Don't try root if src==dst drive or source path given */ >>> if ((drive == srcDrive) >>> || (*srcPath >>> && ((srcPath[1] != ':') || ((srcPath[1] == ':') && srcPath[2])))) >>> *rootPath = '\0'; >>> else >>> sprintf(rootPath, "%c:\\", 'A' + srcDrive); >>- (x || (~x && y)) equal to (x && y), so, there should be: >> /* Don't try root if src==dst drive or source path given */ >> static char root [] = "\0:\\"; >> if (drive != srcDrive && >> (srcPath [0] == '\0' || /* empty string or */ >> (srcPath [1] == ':' && srcPath [2] == '\0'))) /* drive only "A:" */ >> root [0] = 'A' + srcDrive; KJD> I'm glad you can figure what's being checked for, Yes. KJD> ?something about not trying root directory for copies if a path is given Yes. Root is tried only of source ommited or given as drive letter ("A:"), else ("c:\temp", "d:dir", "subdir") only given directory is accessed. KJD> as we may be trying to run sys onto same drive as source using KJD> kernel & command.com contained within a subdirectory of itself This prevented by "srcDrive != drive". Though, I under doubt: may be in case of "" and "D:" always should be checked only root - if you wish to get current directory, you may use "." and "D:."? ------------------------------------------------------- SF.Net is sponsored by: Speed Start Your Linux Apps Now. Build and deploy apps & Web services for Linux with a free DVD software kit from IBM. Click Now! http://ads.osdn.com/?ad_id56&alloc_id438&op=click _______________________________________________ Freedos-kernel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/freedos-kernel