[ECOS] fatfs lseek EOF bug

[ECOS] fatfs lseek EOF bug

[Gratian Crisan]

Hi all,

I think I've found a bug in the ecos FAT implementation for lseek function. 
(fatfs_fo_lseek).
When calling the function like this 'lseek(fd, 0,  SEEK_CUR)' to get the 
current file position and the postion is right at the end of the file the fat 
lseek function returns end of file error EEOF. I've looked at the other 
filesystems from ecos and did a test in linux and the correct behavior seems 
to be to return the current position of the end of file if the read/write 
pointer is at the end of file (equal with the file size).
This bug occurs for example when creating a new file, writing some data in it 
and calling lseek(fd, 0, SEEK_CUR) to get the current file position.

Suggestions?

Thanks,
	Nelu

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] fatfs lseek EOF bug

[Savin Zlobec]

Gratian Crisan wrote:

>Hi all,
>
>I think I've found a bug in the ecos FAT implementation for lseek function. 
>(fatfs_fo_lseek).
>When calling the function like this 'lseek(fd, 0,  SEEK_CUR)' to get the 
>current file position and the postion is right at the end of the file the fat 
>lseek function returns end of file error EEOF. I've looked at the other 
>filesystems from ecos and did a test in linux and the correct behavior seems 
>to be to return the current position of the end of file if the read/write 
>pointer is at the end of file (equal with the file size).
>This bug occurs for example when creating a new file, writing some data in it 
>and calling lseek(fd, 0, SEEK_CUR) to get the current file position.
>
>Suggestions?
>  
>
Hi,

I've tryed to reproduce  the bug you described but without success.
Are you using the latest CVS version of fatfs ?
Can you send me an test case, so I can look into it ?

savin

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] fatfs lseek EOF bug

[Gratian Crisan]

[-- Attachment #1: Type: text/plain, Size: 1578 bytes --]

Hi,

please see my answers below:

On Tuesday 10 May 2005 09:48, Savin Zlobec wrote:
> Gratian Crisan wrote:
> >Hi all,
> >
> >I think I've found a bug in the ecos FAT implementation for lseek
> > function. (fatfs_fo_lseek).
> >When calling the function like this 'lseek(fd, 0,  SEEK_CUR)' to get the
> >current file position and the postion is right at the end of the file the
> > fat lseek function returns end of file error EEOF. I've looked at the
> > other filesystems from ecos and did a test in linux and the correct
> > behavior seems to be to return the current position of the end of file if
> > the read/write pointer is at the end of file (equal with the file size).
> >This bug occurs for example when creating a new file, writing some data in
> > it and calling lseek(fd, 0, SEEK_CUR) to get the current file position.
> >
> >Suggestions?
>
> Hi,
>
> I've tryed to reproduce  the bug you described but without success.
> Are you using the latest CVS version of fatfs ?
***
	Yes.

> Can you send me an test case, so I can look into it ?
***
	See the file attached. The test is for an MMC driver but should be pretty 
straight forward to adapt (only the mount command should be changed).
The test creates a file and writes blocks of data into it while printing the 
current file offset. In a new file after writing the first block, for which 
the value returned is 0, the value returned by lseek function is -1. The 
strange thing is that if you overwrite an existing file the lseek function 
correctly returns the current file offset (equal to EOF).

Thanks,
	Nelu

[-- Attachment #2: mmc_test.c --]
[-- Type: text/x-csrc, Size: 1139 bytes --]

#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <fcntl.h>
#include <sys/stat.h>
#include <errno.h>
#include <string.h>
#include <dirent.h>

#include <cyg/fileio/fileio.h>
#include <cyg/infra/diag.h>

void mmc_write(char fileName[], long nBlocks, long blockSize, unsigned char testChar)
{
	char *buf;
	int i;
	int fd;
	ssize_t wrote;

	if ((buf = (char*)malloc(blockSize)) == NULL) {
		diag_printf("[error] memory allocation failed!\n");
		return;
	}

	memset(buf, testChar, blockSize);

	if ((fd = open(fileName, O_WRONLY|O_CREAT)) < 0){
		diag_printf("[error] opening file %s for write failed!\n", fileName);
		return;
	}

	for(i=0;i<nBlocks;i++) {
		diag_printf("off:%d\n",lseek(fd, 0, SEEK_CUR));

		wrote = write( fd, buf, blockSize );
		if (wrote != blockSize) {
			diag_printf("[warning] incomplete write at block %d\n", i);
		}
	}
	
	close(fd);
}

int main()
{
	int err;
	int i;

	diag_printf("MMC test start ...\n");

	if ((err = mount( "/dev/mmcda0/1", "/", "fatfs" )) < 0) {
		diag_printf("[error] mounting MMC card!\n");
		return 0;
	}
	
	mmc_write("test.txt", 5, 1024, 'x');

	umount( "/" );

	return 1;
}


[-- Attachment #3: Type: text/plain, Size: 148 bytes --]

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] fatfs lseek EOF bug

[Savin Zlobec]

[-- Attachment #1: Type: text/plain, Size: 1509 bytes --]

Gratian Crisan wrote:

>Hi,
>
>please see my answers below:
>
>On Tuesday 10 May 2005 09:48, Savin Zlobec wrote:
>  
>
>>Gratian Crisan wrote:
>>    
>>
>>>Hi all,
>>>
>>>I think I've found a bug in the ecos FAT implementation for lseek
>>>function. (fatfs_fo_lseek).
>>>When calling the function like this 'lseek(fd, 0,  SEEK_CUR)' to get the
>>>current file position and the postion is right at the end of the file the
>>>fat lseek function returns end of file error EEOF. I've looked at the
>>>other filesystems from ecos and did a test in linux and the correct
>>>behavior seems to be to return the current position of the end of file if
>>>the read/write pointer is at the end of file (equal with the file size).
>>>This bug occurs for example when creating a new file, writing some data in
>>>it and calling lseek(fd, 0, SEEK_CUR) to get the current file position.
>>>
>>>Suggestions?
>>>
[snip]

>	See the file attached. The test is for an MMC driver but should be pretty 
>straight forward to adapt (only the mount command should be changed).
>The test creates a file and writes blocks of data into it while printing the 
>current file offset. In a new file after writing the first block, for which 
>the value returned is 0, the value returned by lseek function is -1. The 
>strange thing is that if you overwrite an existing file the lseek function 
>correctly returns the current file offset (equal to EOF).
>  
>
Found the BUG -- Please try the attached patch and let me know if it works.

savin


[-- Attachment #2: fatfs.diff --]
[-- Type: text/plain, Size: 1189 bytes --]

Index: src/fatfs_supp.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/fs/fat/current/src/fatfs_supp.c,v
retrieving revision 1.3
diff -u -r1.3 fatfs_supp.c
--- src/fatfs_supp.c	22 Oct 2004 14:10:23 -0000	1.3
+++ src/fatfs_supp.c	10 May 2005 10:43:14 -0000
@@ -1172,8 +1172,10 @@
     // Err could be EEOF wich means that the given 
     // offset if out of given file (cluster chain)
 
-    if (ENOERR == err)
-        *pos = new_pos; 
+    if (EEOF == err)
+        new_pos.cluster_pos = disk->cluster_size; 
+    
+    *pos = new_pos; 
     
     return err;
 } 
@@ -2056,11 +2058,18 @@
              fatfs_data_pos_t  *pos,
              cyg_uint32         offset)
 {
+    int err;
+    
     CYG_CHECK_DATA_PTRC(disk);
     CYG_CHECK_DATA_PTRC(file);
     CYG_CHECK_DATA_PTRC(pos);
     
-    return get_position_from_off(disk, file->cluster, offset, pos, CO_NONE);
+    err = get_position_from_off(disk, file->cluster, offset, pos, CO_NONE);
+
+    if (EEOF == err && offset == file->size)
+        return ENOERR;
+    else
+        return err;
 }
 
 // -------------------------------------------------------------------------


[-- Attachment #3: Type: text/plain, Size: 148 bytes --]

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] fatfs lseek EOF bug

[Gratian Crisan]

On Tuesday 10 May 2005 13:49, Savin Zlobec wrote:
>
> [snip]
>
> >	See the file attached. The test is for an MMC driver but should be pretty
> >straight forward to adapt (only the mount command should be changed).
> >The test creates a file and writes blocks of data into it while printing
> > the current file offset. In a new file after writing the first block, for
> > which the value returned is 0, the value returned by lseek function is
> > -1. The strange thing is that if you overwrite an existing file the lseek
> > function correctly returns the current file offset (equal to EOF).
>
> Found the BUG -- Please try the attached patch and let me know if it works.
>
> savin

The patch works OK :), thanks.

On a side note: I'm looking for an alternative implementation for 'ftruncate' 
POSIX function which is missing from eCos. Do you know how I could implement 
it for fatfs?

-nelu

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] fatfs lseek EOF bug

[Savin Zlobec]

Gratian Crisan wrote:

> The patch works OK :), thanks.

Ok, I'll wait a couple of days and then send the patch to ecos-patches.

>On a side note: I'm looking for an alternative implementation for 'ftruncate' 
>POSIX function which is missing from eCos. Do you know how I could implement 
>it for fatfs?
>  
>
It should not be too hard - If the file is shrinking find the last 
cluster of the new file length
(get_position_from_off), free all clusters from the current one the the 
end of the file
(free_cluster_chain) and change the size of the coresponding fatfs node. 
If the file is extending,
then you need to allocate the appropriate number of clusters and link 
them into the file cluster chain
(find_and_append_cluster).

savin

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss