Hi!
16-Июн-2006 06:46 [EMAIL PROTECTED] (Blair Campbell) wrote to
[EMAIL PROTECTED]:
BC> portable to OpenWatcom. Optimizations are welcome.
BC> --- NEW FILE: lfnfuncs.c ---
BC> char * getshortfilename( const char *longfilename )
BC> static char shortfilename[ 128 ];
BC> _DS = FP_SEG( longfilename );
Bug! _DS modified, but not preserved around calls to INT21! (And in
lfnfuncs.c are more such places).
BC> if( _AX == 0x7100 || _CFLAG ) return( NULL );
Bug! First comparision clears Carry flag! So, this should look so:
if (_CFLAG || _AX == 0x7100) return NULL;
(And in lfnfuncs.c are more such places). And about `return' see below.
BC> return( shortfilename );
BC> int lfn_chmod( const char *filename, int func, ... )
BC> char *sfn = getshortfilename( filename );
BC> if( sfn == NULL ) sfn = filename;
1. Bug: you can't assign pointer-to-const to pointer-to-non-const.
2. _All_ calls to getshortfilename() followed by reassign sfn, if it NULL.
So, why no write so:
const char * getSFN (const char *name) {
/*^^^^^*/
static char SFN [128];
[...]
if (_CFLAG == 0 && _AX != 0x7100) name = SFN;
return LFN;
?
BC> va_start( vargs, func );
BC> if( func )attrib = va_arg( vargs, int );
BC> va_end( vargs );
BC> return( _chmod( sfn, func, attrib ) );
BC> }
With given above modification for getSFN(), this lfn_chmod() function
may be noticeably simplified:
int lfn_chmod (const char *name, int func, ...) {
va_start (vargs, func);
if (func) attrib = va_arg (vargs, int);
va_end (vargs);
return _chmod (getSFN (name), func, attrib));
}
Same for other functions:
BC> int lfnchmod( const char *filename, int mode )
BC> {
BC> char *sfn = getshortfilename( filename );
BC> if( sfn == NULL ) sfn = filename;
BC> return( chmod( sfn, mode ) );
BC> }
int lfnchmod (const char *name, int mode) {
return chmod (getSFN (name), mode);
}
The more so, such plain return may be eliminated by inline-ing directly into
header:
int lfnchmod (const char *name, int mode);
#define lfnchmod(name, mode) chmod (getSFN (name), mode)
BC> int lfnaccess( const char *filename, int mode )
BC> {
BC> int attribs = lfn_chmod( filename, 0 );
BC> return( ( attribs == -1 ) ? -1 : ( mode == 0 || mode == 1 || mode == 4 )
BC> ?
BC> 0 : ( ( mode == 2 || mode == 6 ) && !( attribs & FA_RDONLY ) ) ?
BC> 0 :
BC> -1 );
BC> }
Well... This definitely should be made more readable (and readability
allows to optimize this code):
int lfnaccess (const char *name, int mode) {
int attrib = lfn_chmod (name, 0);
if (attrib != -1 &&
((mode & 2) == 0 || (attrib & FA_RDONLY) == 0)) return 0;
return -1;
}
BC> char * lfn_getdcwd( int drive, char *buffer, int size ) {
BC> char *tmpbuf;
BC> static char longfilename[ 270 ];
BC> if( !buffer ) tmpbuf = _getdcwd( drive, NULL, size );
BC> else if( _getdcwd( drive, tmpbuf, size ) == NULL ) return( NULL );
----------------------------------^^^^^^
Bug! There used random value.
BC> if( tmpbuf == NULL ) return( NULL );
Same here: if buffer is non-NULL, then tmpbuf value is undefined.
Probably, above should be next code:
char * lfn_getdcwd (int drive, char *buffer, int size) {
static char LFN [270];
char *tmpbuf = _getdcwd (drive, buffer, size);
if (tmpbuf == NULL) return NULL;
BC> if( _AX == 0x7100 || _CFLAG ) return( ( buffer = tmpbuf ) );
Bug! See explanation above for getSFN(). Also, "buffer=" here is
redundant.
BC> if( size < strlen( longfilename ) ) {
Bug! Should be "<=".
BC> errno = ERANGE;
BC> return( NULL );
BC> }
BC> if( ( buffer = malloc( strlen( longfilename ) + 1 ) ) == NULL ) {
BC> errno = ENOMEM;
BC> return( NULL );
BC> }
BC> strcpy( buffer, longfilename );
BC> return( buffer );
Multiple bugs! If originally `buffer' is NULL, then allocated memory
by _getdcwd(), but there it not freed! And vice versa: if `buffer' isn't
NULL, then anyway result is placed in heap, not in client buffer. So, I
suggest, this code should look so:
size_t LFN_len;
if (buffer == NULL) free (tmpbuf);
LFN_len = strlen (LFN);
if (size <= LFN_len) {
errno = ERANGE;
return NULL;
}
if (buffer == NULL && (buffer = malloc (LFN_len + 1)) == NULL) {
errno = ENOMEM;
return NULL;
}
return strcpy (buffer, LFN);
_______________________________________________
Freedos-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/freedos-devel
