From 0c553faff1e41be9e7bd8eb6231584039694d07d Mon Sep 17 00:00:00 2001 From: Olaf Barthel Date: Sun, 24 Apr 2005 09:53:12 +0000 Subject: [PATCH] - Changed how errors are detected, as returned by Write(), Read() and Seek(). Seek() is particularly challenging because the value it returns might be a valid file position and not an error. - Replaced numeric function return codes of 0 and -1 with macros OK, SEEK_ERROR/ERROR to clarify the respective purposes. - Changed how ftell() and fseek() are used, double-checking the return value and the errno code. git-svn-id: file:///Users/olsen/Code/migration-svn-zu-git/logical-line-staging/clib2/trunk@14923 87f5fb63-7c3d-0410-a384-fd976d0f7a62 --- library/changes | 11 ++++++++++ library/fcntl_lseek.c | 14 ++++++++++-- library/stdio_dropiobreadbuffer.c | 16 ++++++++++++-- library/stdio_fdhookentry.c | 31 +++++++++++++++++++------- library/stdio_fgetpos.c | 4 ++-- library/stdio_fseek.c | 18 +++++++++++++--- library/stdio_fsetpos.c | 4 ++-- library/stdio_ftell.c | 14 ++++++++++-- library/stdio_grow_file.c | 17 +++++++++------ library/stdio_record_locking.c | 23 ++++++++++---------- library/stdio_rename.c | 4 ++-- library/unistd_ftruncate.c | 36 ++++++++++++++++--------------- 12 files changed, 135 insertions(+), 57 deletions(-) diff --git a/library/changes b/library/changes index a316c6f..80b16a6 100644 --- a/library/changes +++ b/library/changes @@ -1,3 +1,14 @@ +- Changed how errors are detected, as returned by Write(), Read() and + Seek(). Seek() is particularly challenging because the value it + returns might be a valid file position and not an error. + +- Replaced numeric function return codes of 0 and -1 with macros OK, + SEEK_ERROR/ERROR to clarify the respective purposes. + +- Changed how ftell() and fseek() are used, double-checking the return + value and the errno code. + + c.lib 1.191 (9.4.2005) - The name of the public record locking semaphore has to be preallocated diff --git a/library/fcntl_lseek.c b/library/fcntl_lseek.c index 4288759..f19218a 100644 --- a/library/fcntl_lseek.c +++ b/library/fcntl_lseek.c @@ -1,5 +1,5 @@ /* - * $Id: fcntl_lseek.c,v 1.8 2005-04-24 08:46:37 obarthel Exp $ + * $Id: fcntl_lseek.c,v 1.9 2005-04-24 09:53:11 obarthel Exp $ * * :ts=4 * @@ -83,13 +83,23 @@ lseek(int file_descriptor, off_t offset, int mode) assert( fd->fd_Action != NULL ); + /* Note that a return value of -1 (= SEEK_ERROR) may be a + valid file position in files larger than 2 GBytes. Just + to be sure, we therefore also check the secondary error + to verify that what could be a file position is really + an error indication. */ position = (*fd->fd_Action)(fd,&fam); - if(position == EOF) + if(position == SEEK_ERROR && fam.fam_Error != OK) { __set_errno(fam.fam_Error); goto out; } + /* If this is a valid file position, clear 'errno' so that + it cannot be mistaken for an error. */ + if(position < 0) + __set_errno(OK); + result = position; out: diff --git a/library/stdio_dropiobreadbuffer.c b/library/stdio_dropiobreadbuffer.c index 3ab3f37..bdea43e 100644 --- a/library/stdio_dropiobreadbuffer.c +++ b/library/stdio_dropiobreadbuffer.c @@ -1,5 +1,5 @@ /* - * $Id: stdio_dropiobreadbuffer.c,v 1.7 2005-04-24 08:46:37 obarthel Exp $ + * $Id: stdio_dropiobreadbuffer.c,v 1.8 2005-04-24 09:53:11 obarthel Exp $ * * :ts=4 * @@ -74,6 +74,7 @@ __drop_iob_read_buffer(struct iob * file) if(num_unread_bytes > 0) { struct file_action_message fam; + LONG position; SHOWMSG("calling the action function"); @@ -83,7 +84,13 @@ __drop_iob_read_buffer(struct iob * file) assert( file->iob_Action != NULL ); - if((*file->iob_Action)(file,&fam) == EOF) + /* Note that a return value of -1 (= SEEK_ERROR) may be a + valid file position in files larger than 2 GBytes. Just + to be sure, we therefore also check the secondary error + to verify that what could be a file position is really + an error indication. */ + position = (*file->iob_Action)(file,&fam); + if(position == SEEK_ERROR && fam.fam_Error != OK) { SHOWMSG("that didn't work"); @@ -95,6 +102,11 @@ __drop_iob_read_buffer(struct iob * file) goto out; } + + /* If this is a valid file position, clear 'errno' so that + it cannot be mistaken for an error. */ + if(position < 0) + __set_errno(OK); } file->iob_BufferReadBytes = 0; diff --git a/library/stdio_fdhookentry.c b/library/stdio_fdhookentry.c index d7a41de..61245b7 100644 --- a/library/stdio_fdhookentry.c +++ b/library/stdio_fdhookentry.c @@ -1,5 +1,5 @@ /* - * $Id: stdio_fdhookentry.c,v 1.29 2005-04-24 08:46:37 obarthel Exp $ + * $Id: stdio_fdhookentry.c,v 1.30 2005-04-24 09:53:11 obarthel Exp $ * * :ts=4 * @@ -192,8 +192,11 @@ __fd_hook_entry( PROFILE_OFF(); + /* Make sure that if we get a value of -1 out of Seek() + to check whether this was an error or a numeric + overflow. */ position = Seek(file,0,OFFSET_END); - if(position != SEEK_ERROR) + if(position != SEEK_ERROR || IoErr() == OK) { if(FLAG_IS_SET(fd->fd_Flags,FDF_CACHE_POSITION)) fd->fd_Position = Seek(file,0,OFFSET_CURRENT); @@ -444,19 +447,28 @@ __fd_hook_entry( if(FLAG_IS_SET(fd->fd_Flags,FDF_CACHE_POSITION)) { - current_position = fd->fd_Position; + current_position = (off_t)fd->fd_Position; } else { + LONG position; + PROFILE_OFF(); - current_position = Seek(file,0,OFFSET_CURRENT); + position = Seek(file,0,OFFSET_CURRENT); PROFILE_ON(); - if(current_position == SEEK_ERROR) + /* Note that a return value of -1 (= SEEK_ERROR) may be a + valid file position in files larger than 2 GBytes. Just + to be sure, we therefore also check the secondary error + to verify that what could be a file position is really + an error indication. */ + if(position == SEEK_ERROR && IoErr() != OK) { fam->fam_Error = EBADF; goto out; } + + current_position = (off_t)position; } new_position = current_position; @@ -493,7 +505,10 @@ __fd_hook_entry( position = Seek(file,fam->fam_Offset,new_mode); PROFILE_ON(); - if(position == SEEK_ERROR) + /* Same as above: verify that what we got out of + Seek() is really an error and not a valid + file position. */ + if(position == SEEK_ERROR && IoErr() != OK) { D(("seek failed, fam->fam_Mode=%ld (%ld), offset=%ld, ioerr=%ld",new_mode,fam->fam_Mode,fam->fam_Offset,IoErr())); @@ -505,11 +520,11 @@ __fd_hook_entry( the new file position. First, we need to find out if the file is really shorter than required. If not, then it must have been a different error. */ - if((NOT fib_is_valid && CANNOT __safe_examine_file_handle(file,fib)) || (new_position <= fib->fib_Size)) + if((NOT fib_is_valid && CANNOT __safe_examine_file_handle(file,fib)) || (new_position <= (off_t)fib->fib_Size)) goto out; /* Now try to make that file larger. */ - if(__grow_file_size(fd,new_position - fib->fib_Size) < 0) + if(__grow_file_size(fd,new_position - (off_t)fib->fib_Size) < 0) { fam->fam_Error = __translate_io_error_to_errno(IoErr()); goto out; diff --git a/library/stdio_fgetpos.c b/library/stdio_fgetpos.c index 5f86734..58e373b 100644 --- a/library/stdio_fgetpos.c +++ b/library/stdio_fgetpos.c @@ -1,5 +1,5 @@ /* - * $Id: stdio_fgetpos.c,v 1.5 2005-04-24 08:46:37 obarthel Exp $ + * $Id: stdio_fgetpos.c,v 1.6 2005-04-24 09:53:12 obarthel Exp $ * * :ts=4 * @@ -74,7 +74,7 @@ fgetpos(FILE *stream, fpos_t *pos) #endif /* CHECK_FOR_NULL_POINTERS */ position = ftell(stream); - if(position == SEEK_ERROR) + if(position == SEEK_ERROR && __get_errno() != OK) { SHOWMSG("ftell() didn't work."); diff --git a/library/stdio_fseek.c b/library/stdio_fseek.c index c5d529f..e854aeb 100644 --- a/library/stdio_fseek.c +++ b/library/stdio_fseek.c @@ -1,5 +1,5 @@ /* - * $Id: stdio_fseek.c,v 1.8 2005-04-24 08:46:37 obarthel Exp $ + * $Id: stdio_fseek.c,v 1.9 2005-04-24 09:53:12 obarthel Exp $ * * :ts=4 * @@ -108,7 +108,7 @@ fseek(FILE *stream, long int offset, int wherefrom) long int current_position; current_position = ftell(stream); - if(current_position >= 0) + if(current_position != SEEK_ERROR || __get_errno() == OK) { offset -= current_position; @@ -143,6 +143,7 @@ fseek(FILE *stream, long int offset, int wherefrom) if(NOT buffer_position_adjusted) { struct file_action_message fam; + LONG position; /* Oh dear, no luck. So we have to get rid of the * current buffer contents and start with a clean @@ -175,7 +176,13 @@ fseek(FILE *stream, long int offset, int wherefrom) assert( file->iob_Action != NULL ); - if((*file->iob_Action)(file,&fam) == EOF) + /* Note that a return value of -1 (= SEEK_ERROR) may be a + valid file position in files larger than 2 GBytes. Just + to be sure, we therefore also check the secondary error + to verify that what could be a file position is really + an error indication. */ + position = (*file->iob_Action)(file,&fam); + if(position == SEEK_ERROR && fam.fam_Error != OK) { SET_FLAG(file->iob_Flags,IOBF_ERROR); @@ -183,6 +190,11 @@ fseek(FILE *stream, long int offset, int wherefrom) goto out; } + + /* If this is a valid file position, clear 'errno' so that + it cannot be mistaken for an error. */ + if(position < 0) + __set_errno(OK); } } diff --git a/library/stdio_fsetpos.c b/library/stdio_fsetpos.c index b3af081..4eaf87e 100644 --- a/library/stdio_fsetpos.c +++ b/library/stdio_fsetpos.c @@ -1,5 +1,5 @@ /* - * $Id: stdio_fsetpos.c,v 1.6 2005-04-24 08:46:37 obarthel Exp $ + * $Id: stdio_fsetpos.c,v 1.7 2005-04-24 09:53:12 obarthel Exp $ * * :ts=4 * @@ -72,7 +72,7 @@ fsetpos(FILE *stream, fpos_t *pos) } #endif /* CHECK_FOR_NULL_POINTERS */ - if(fseek(stream,(long int)(*pos),SEEK_SET) != 0) + if(fseek(stream,(long int)(*pos),SEEK_SET) == SEEK_ERROR && __get_errno() != OK) { SHOWMSG("fseek failed"); goto out; diff --git a/library/stdio_ftell.c b/library/stdio_ftell.c index c789bb2..19ef4e1 100644 --- a/library/stdio_ftell.c +++ b/library/stdio_ftell.c @@ -1,5 +1,5 @@ /* - * $Id: stdio_ftell.c,v 1.8 2005-04-24 08:46:37 obarthel Exp $ + * $Id: stdio_ftell.c,v 1.9 2005-04-24 09:53:12 obarthel Exp $ * * :ts=4 * @@ -96,8 +96,13 @@ ftell(FILE *stream) assert( file->iob_Action != NULL ); + /* Note that a return value of -1 (= SEEK_ERROR) may be a + valid file position in files larger than 2 GBytes. Just + to be sure, we therefore also check the secondary error + to verify that what could be a file position is really + an error indication. */ position = (*file->iob_Action)(file,&fam); - if(position == EOF) + if(position == SEEK_ERROR && fam.fam_Error != OK) { SET_FLAG(file->iob_Flags,IOBF_ERROR); @@ -106,6 +111,11 @@ ftell(FILE *stream) goto out; } + /* If this is a valid file position, clear 'errno' so that + it cannot be mistaken for an error. */ + if(position < 0) + __set_errno(OK); + if(__iob_read_buffer_is_valid(file)) { /* Subtract the number of bytes still in the buffer which have diff --git a/library/stdio_grow_file.c b/library/stdio_grow_file.c index 3a98ffb..0444b69 100644 --- a/library/stdio_grow_file.c +++ b/library/stdio_grow_file.c @@ -1,5 +1,5 @@ /* - * $Id: stdio_grow_file.c,v 1.4 2005-04-24 08:46:37 obarthel Exp $ + * $Id: stdio_grow_file.c,v 1.5 2005-04-24 09:53:12 obarthel Exp $ * * :ts=4 * @@ -54,8 +54,9 @@ __grow_file_size(struct fd * fd,int num_bytes) int bytes_written; int buffer_size; int size; - int position; - int current_position; + LONG seek_position; + off_t position; + off_t current_position; int alignment_skip; int result = ERROR; @@ -104,19 +105,23 @@ __grow_file_size(struct fd * fd,int num_bytes) memset(aligned_buffer,0,(size_t)buffer_size); PROFILE_OFF(); - position = Seek(fd->fd_DefaultFile,0,OFFSET_END); + seek_position = Seek(fd->fd_DefaultFile,0,OFFSET_END); PROFILE_ON(); - if(position == SEEK_ERROR) + if(seek_position == SEEK_ERROR && IoErr() != OK) { SHOWMSG("could not move to the end of the file"); goto out; } + position = (off_t)seek_position; + PROFILE_OFF(); - current_position = Seek(fd->fd_DefaultFile,0,OFFSET_CURRENT); + seek_position = Seek(fd->fd_DefaultFile,0,OFFSET_CURRENT); PROFILE_ON(); + current_position = (off_t)seek_position; + /* Try to make the first write access align the file position * to a block offset. Subsequent writes will then access the * file at positions that are multiples of the block size. diff --git a/library/stdio_record_locking.c b/library/stdio_record_locking.c index 3ea093f..32fc621 100644 --- a/library/stdio_record_locking.c +++ b/library/stdio_record_locking.c @@ -1,5 +1,5 @@ /* - * $Id: stdio_record_locking.c,v 1.10 2005-04-24 08:46:37 obarthel Exp $ + * $Id: stdio_record_locking.c,v 1.11 2005-04-24 09:53:12 obarthel Exp $ * * :ts=4 * @@ -746,13 +746,14 @@ __handle_record_locking(int cmd,struct flock * l,struct fd * fd,int * error_ptr) D_S(struct FileInfoBlock,fib); BOOL fib_is_valid = FALSE; BPTR parent_dir = ZERO; - LONG current_position; + LONG seek_position; + off_t current_position; int result = ERROR; - LONG original_len; + off_t original_len; LONG error = OK; - LONG start = 0; - LONG len = 0; - LONG stop; + off_t start = 0; + off_t len = 0; + off_t stop; ENTER(); @@ -828,12 +829,10 @@ __handle_record_locking(int cmd,struct flock * l,struct fd * fd,int * error_ptr) SHOWMSG("SEEK_CUR"); PROFILE_OFF(); - - current_position = Seek(file_handle,0,OFFSET_CURRENT); - + seek_position = Seek(file_handle,0,OFFSET_CURRENT); PROFILE_ON(); - if(current_position == SEEK_ERROR) + if(seek_position == SEEK_ERROR && IoErr() != OK) { SHOWMSG("could not obtain current seek position"); @@ -841,6 +840,8 @@ __handle_record_locking(int cmd,struct flock * l,struct fd * fd,int * error_ptr) goto out; } + current_position = (off_t)seek_position; + start = current_position + l->l_start; if(l->l_len == 0) @@ -867,7 +868,7 @@ __handle_record_locking(int cmd,struct flock * l,struct fd * fd,int * error_ptr) fib_is_valid = TRUE; - start = fib->fib_Size + l->l_start; + start = (off_t)fib->fib_Size + l->l_start; if(l->l_len == 0) len = LONG_MAX; diff --git a/library/stdio_rename.c b/library/stdio_rename.c index 348f393..d359650 100644 --- a/library/stdio_rename.c +++ b/library/stdio_rename.c @@ -1,5 +1,5 @@ /* - * $Id: stdio_rename.c,v 1.7 2005-04-24 08:46:37 obarthel Exp $ + * $Id: stdio_rename.c,v 1.8 2005-04-24 09:53:12 obarthel Exp $ * * :ts=4 * @@ -123,7 +123,7 @@ rename(const char *oldname,const char *newname) if(CANNOT DeleteFile((STRPTR)newname)) { - SHOWMS("couldn't delete the file"); + SHOWMSG("couldn't delete the file"); __set_errno(__translate_io_error_to_errno(error)); goto out; diff --git a/library/unistd_ftruncate.c b/library/unistd_ftruncate.c index 6d5f073..b8bb62b 100644 --- a/library/unistd_ftruncate.c +++ b/library/unistd_ftruncate.c @@ -1,5 +1,5 @@ /* - * $Id: unistd_ftruncate.c,v 1.11 2005-04-24 08:46:37 obarthel Exp $ + * $Id: unistd_ftruncate.c,v 1.12 2005-04-24 09:53:12 obarthel Exp $ * * :ts=4 * @@ -47,9 +47,9 @@ ftruncate(int file_descriptor, off_t length) D_S(struct FileInfoBlock,fib); int result = ERROR; struct fd * fd = NULL; - BOOL restore_initial_position = FALSE; off_t current_file_size; - off_t initial_position = -1; + LONG initial_position = 0; + BOOL initial_position_valid = FALSE; ENTER(); @@ -113,21 +113,23 @@ ftruncate(int file_descriptor, off_t length) goto out; } - current_file_size = fib->fib_Size; + current_file_size = (off_t)fib->fib_Size; /* Is the file to be made shorter than it is right now? */ if(length < current_file_size) { /* Remember where we started. */ - if(initial_position < 0) + if(NOT initial_position_valid) { initial_position = Seek(fd->fd_DefaultFile,0,OFFSET_CURRENT); - if(initial_position == SEEK_ERROR) + if(initial_position == SEEK_ERROR && IoErr() != OK) goto out; + + initial_position_valid = TRUE; } /* Careful: seek to a position where the file can be safely truncated. */ - if(Seek(fd->fd_DefaultFile,length,OFFSET_BEGINNING) == SEEK_ERROR) + if(Seek(fd->fd_DefaultFile,length,OFFSET_BEGINNING) == SEEK_ERROR && IoErr() != OK) { D(("could not move to file offset %ld",length)); @@ -146,29 +148,29 @@ ftruncate(int file_descriptor, off_t length) /* If the file is now shorter than the file position, which must not be changed by a call to ftruncate(), extend the file again, filling the extension with 0 bytes. */ - if(initial_position > length) + if((off_t)initial_position > length) { current_file_size = length; - length = initial_position; + length = (off_t)initial_position; } - - restore_initial_position = TRUE; } /* Is the size of the file to grow? */ if(length > current_file_size) { /* Remember where we started. */ - if(initial_position < 0) + if(NOT initial_position_valid) { initial_position = Seek(fd->fd_DefaultFile,0,OFFSET_CURRENT); - if(initial_position == SEEK_ERROR) + if(initial_position == SEEK_ERROR && IoErr() != OK) goto out; + + initial_position_valid = TRUE; } /* Move to what should be the end of the file. */ - if(Seek(fd->fd_DefaultFile,current_file_size,OFFSET_BEGINNING) == SEEK_ERROR) + if(Seek(fd->fd_DefaultFile,current_file_size,OFFSET_BEGINNING) == SEEK_ERROR && IoErr() != OK) { D(("could not move to file offset %ld",current_file_size)); @@ -185,15 +187,15 @@ ftruncate(int file_descriptor, off_t length) __set_errno(__translate_io_error_to_errno(IoErr())); goto out; } - - restore_initial_position = TRUE; } result = OK; out: - if(restore_initial_position) + /* ftruncate() may change the size of the file, but it may + not change the current file position. */ + if(initial_position_valid) Seek(fd->fd_DefaultFile,initial_position,OFFSET_CURRENT); __fd_unlock(fd);