iOS Binary file read issue

[Angry Bugs]

Hi!,

I use the following code to open a binary file:

const char* fileName = "myFile.bin";	

FILE* file = NULL;


const char* buffer;

NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
NSString *path = [NSString stringWithUTF8String:fileName];
NSArray *paths = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES);
NSString *documentsDirectory = [paths objectAtIndex:0];
path = [documentsDirectory stringByAppendingPathComponent:path];
	
if (path != nil)
    buffer = [path UTF8String];


[pool release];

file = fopen(buffer, "rb");
	
if (!file)
    return -1;

//Do fread stuff here

fclose(file);

return 0;

But sometimes, not very often, the functions returns -1.
Is there any problem in this code, or a reason why only sometimes this function doesn't find the file?, although the file is present.

Any help would be appreciated.

Thanks.

 

[chown33]

1. Go to the NSString class reference doc. Find the description of the UTF8String method. Read what it says about "the autorelease context in which the C string is created."


2. When referring to file paths, you should always use the file-system representation. There are two NSString methods related to this. I recommend the one that copies the data into your provided buffer.

 

[Angry Bugs]

Thanks a lot.

I think replacing this :

buffer = [path UTF8String];

with this:

sprintf(buffer, "%s", [path UTF8String]);

will do the job.

 

[chown33]

I think replacing this :

buffer = [path UTF8String];

with this:

sprintf(buffer, "%s", [path UTF8String]);

will do the job.

If that's the only change you make, then you haven't fixed anything. You've just traded one bug for another.

The posted sprintf() is just a strange way of performing an strcpy(). I advise you to look up strcpy() to see what it does. I also advise looking at strncpy() to see how it differs, and compare to snprintf(). Consider what might happen if you copy a string that's longer than the buffer memory reserved for it.

At the point you propose placing that code, what is buffer pointing to? Is it a valid allocated block of memory? Of what length? I suggest using the debugger to step through and see what buffer holds at that point. If buffer isn't pointing at a valid block of memory, what do you suppose will happen if you copy a string into that memory?


And you're still making the mistake of using UTF8String instead of one of the file-system representation methods. See this post on Apple's cocoa-dev list:
http://lists.apple.com/archives/cocoa-dev/2012/Aug/msg00821.html

 

[PhoneyDeveloper]

Use [path fileSystemRepresentation]

Is it possible that your file names have non-ascii characters in them?

Get that [pool release] out of there. You are making your path go away!

 

[Angry Bugs]

Thanks for taking the time to reply.

I read the link you post, and the changed the code.
I'm not sure that the length of the buffer is enough.

const char* fileName = "myFile.bin";	

FILE* file = NULL;


char buffer[256];

NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
NSString *path = [NSString stringWithUTF8String:fileName];
NSArray *paths = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES);
NSString *documentsDirectory = [paths objectAtIndex:0];
path = [documentsDirectory stringByAppendingPathComponent:path];

BOOL success;
	
if (path != nil)
    success = [path getFileSystemRepresentation:buffer maxLength:256]

if (!success)
    return -1;

[pool release];

file = fopen(buffer, "rb");
	
if (!file)
    return -1;

//Do fread stuff here

fclose(file);

return 0;

 

[Angry Bugs]

Use [path fileSystemRepresentation]

Is it possible that your file names have non-ascii characters in them?

Get that [pool release] out of there. You are making your path go away!

All the characters are ascii, i think the other way could be to use fileSystemRepresentation and move [pool release] to the end of the function, right?

 

[chown33]

All the characters are ascii, i think the other way could be to use fileSystemRepresentation and move [pool release] to the end of the function, right?

Why are you so concerned about memory usage that you feel the need to bracket a few strings with their own auto-release pool?


If you're looking for waste, consider the red-hilited code:

[COLOR="Red"]const char* fileName = "myFile.bin";	[/COLOR]

FILE* file = NULL;

char buffer[256];

NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
[COLOR="red"]NSString *path = [NSString stringWithUTF8String:fileName];[/COLOR]
NSArray *paths = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES);
NSString *documentsDirectory = [paths objectAtIndex:0];
[COLOR="red"]path = [documentsDirectory stringByAppendingPathComponent:path];[/COLOR]

You're starting with a constant C string, converting that to an immutable NSString, then appending that to another NSString. Why all the conversions? Do you realize that Objective-C supports NSString literals? For example, all the red can be replaced by one green line:

NSArray *paths = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES);
NSString *documentsDirectory = [paths objectAtIndex:0];
[COLOR="SeaGreen"]NSString *path = [documentsDirectory stringByAppendingPathComponent:@"myFile.bin"];[/COLOR]

Notice that I also removed the extra auto-release pool, so now you can just use fileSystemRepresentation without having to copy it or worry that your local buffer isn't long enough. All the objects that were made will be auto-released when the existing pool is drained. No micro-managing of memory is necessary.


As another possible bug, consider what happens if path is nil in your code. If you haven't done a static analysis, I think you'll find a reference to an uninitialized buffer being used by fopen(). You may even find that the compiler is giving you a warning about this.

 

[KnightWRX]

Why are you so concerned about memory usage that you feel the need to bracket a few strings with their own auto-release pool?


If you're looking for waste, consider the red-hilited code:

[COLOR="Red"]const char* fileName = "myFile.bin";	[/COLOR]

FILE* file = NULL;

char buffer[256];

NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
[COLOR="red"]NSString *path = [NSString stringWithUTF8String:fileName];[/COLOR]
NSArray *paths = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES);
NSString *documentsDirectory = [paths objectAtIndex:0];
[COLOR="red"]path = [documentsDirectory stringByAppendingPathComponent:path];[/COLOR]

If we're talking waste, how bout plain ol' :

FILE * file;
NSString * path = [[NSBundle mainBundle] pathForResource: @"myFile" ofType: @"bin"];
if(path)
{
      file = fopen([path fileSystemRepresentation], "rb");

      if(file)
      {
            /* Do Stuff */
            fclose(file);
      }
}

No need for 50 URLs/paths/strings here, strictly speaking if we're discussing iOS programming. I don't think you have access outside your app bundle anyhow.

 

[Angry Bugs]

As another possible bug, consider what happens if path is nil in your code. If you haven't done a static analysis, I think you'll find a reference to an uninitialized buffer being used by fopen(). You may even find that the compiler is giving you a warning about this.

You're right.

Again, thanks a lot for your reply, it really helps me a lot.