C code prob

[Jherek Carnelia]

I'm trying to learn C and am stuck on what (at least to) me seems a very simple bit of code. Any ideas what I'm doing wrong??

/* ***************************************** */
/* returning a file pointer from a function */

#include <stdio.h>
#include <stdlib.h>

FILE *source; /* pointer to a file structure */

FILE *open_file(void); /* function to return a file pointer */

main()
{
source = open_file();

if(source != NULL)
/* this line is never printed */
printf("from main file opened\n");

fclose(source);
getch();
return 0;
}

FILE *open_file(void)
{
FILE *s;

printf("in function\n");
if(s=fopen("test.txt", "wb") == NULL)
{
printf("cant open");
return s;
}
/* this line prints and the file is created */
printf("from function file opened\n");
return s;
}

I get a warning when I build the code on this line:

if(s=fopen("test.txt", "wb") == NULL)
pointer type mismatch "FILE *" does not match "int"
and the value returned from the function is NULL

[eLowar]

It's a problem with operator precedence. What your code means is:

if ( s = (fopen("test.txt", "wb") == NULL) )

Note that the highlighted (red) parentheses are added implicitly. The result of the comparison (==) operator is of type int, which you then assign to s of type FILE*, hence the warning. What you meant is:

if ( (s = fopen("test.txt", "wb")) == NULL )

Note that you have to add the highlighted (blue) parantheses explicitly. The result of the assignment (=) is the value on the right side of the operator, which is of type FILE*, which you then compare with NULL, which you can compare with any pointer type.

The lesson here being that for practical reasons, = has a very low priority among the operators. Think of it as + vs. * in maths, e.g. a + b * c. That too is like a + (b * c), and if you want (a + b) * c you have to write so explicitly.

Hope this helps and good luck with your studies.

[Calgor]

When I was learning C, I found it always handy to have a reference table for operator precedence. For example:
http://www.difranco.net/cop2220/op-prec.htm

On the webpage, you will see that == is higher in the table than =, just as eLowar has explained. Any C learning book worth its salt should also have such a table.

EDIT: And when in doubt, I put parentheses around to group exactly what I want.

[eLowar]

Very nice link, I was trying to find something like that online to post here.

[Jherek Carnelia]

Doh! I've been tearing my hair out over that!! I just couldn't see it! Thanks guys!

[thomas]

if ( (s = fopen("test.txt", "wb")) == NULL )

As a beginner you should always write it like this:

s = fopen("test.txt", "wb");
if (s == NULL)

Also this part of your code looks a bit funny:

if (s == NULL)
{
printf("cant open");
return s;
}

You test if s is NULL and if so, you return s. You could as well return NULL, just to make it a bit more readable.

[Zetr0]

Its a good start!

just a couple of syntax problems and its away...



firstly i will mention that global FILE pointers are asking for trouble LOL so its best not to use them.. just declare thier use in each function parsing if need be

such that

void main( void )
{
	FILE *fp;


	if( !open_file("text.txt", fp) ) // error occured
	{
		printf("\n\nError opening file...\n\n");
		exit(0);// you dont need to use this its just a habbit of mine!
	}
	else
	{
		/*


			nice lump of code here		

	
		*/
        fclose( fp ); // always close the FILE pointer after use
	}
}


bool open_file( char *filename, FILE *fp )
{
 	fp = fopen( filename, "rb" );  // rb = mode read binary

	if( fp != NULL )
		return true;
	else
		return false;
}

[eLowar]

Originally Posted by Zetr0

bool open_file( char *filename, FILE *fp )
{
 	fp = fopen( filename, "rb" );  // rb = mode read binary

	if( fp != NULL )
		return true;
	else
		return false;
}

A few problems with that.

For starters there is no pre-defined bool type in C (there is in C++, and many C libraries define a bool or BOOL type, but it's not built into C), also no true or false.

Also this will just overwrite a local copy of the file pointer with the result of fopen, with no effect on the variable outside that function whatsoever. If you want to use an output parameter like that in C (in C++ there are references), you have to pass a pointer to the type you want to overwrite and then dereference it when assigning. Like so:

int open_file( char *filename, FILE **fp )
{
	/* [...] */
	*fp = fopen( filename, "rb" );  /* rb = mode read binary */
	/* [...] */
}

In C parameters are always passed by value.

That said, I agree, global variables are a bad idea and will likely get you in trouble in the future if you get into the habit of using them without thinking about it. Make sure you understand variable scopes and their consequences before going on.

That's all from me for now.

[Zetr0]

damn M$ i am getting soft!

[eLowar]

No // style comments either, btw.

Not officially until C99 at any rate, which does also have a _Bool type. If you want to use "bool" you have to #include <stdbool.h> though.

I love people talking about languages I know instead of all that assembler talk I can contribute nothing to.

[Jherek Carnelia]

The point of my previous post was to enable me to write the following little prog. As noted, I owe a debt to Thomas Nokielski for his copdis routine and structure (and also you guys for your help!). I needed something that would allow me to read the output from AR3 save memory dumps. It seemed a good project to try and learn C for...

As it is my first real C prog I am fairly pleased with it, although I am absolutely sure it could have been done far more elegantly than the way I have...

Anyway, for those that are interested here it is...

[eLowar]

At a quick glance it looks pretty clean.

Unfortunately I can't test it because I don't have an Amiga development environment installed. Next task for you: portable version.

[Jherek Carnelia]

Hmm. How to make it portable when I use Amiga Libraries? (although to be fair, I never intended to use it anywhere else than in WinUAE).

I guess that filerequesters are out! I will have to study the standard libs and see what's available. Without looking, I suppose it would have to use command line arguments (which is what I was trying to avoid!)

Still, a good exercise, I will see what I can do - although as I'm going out tonight (and beer and programming don't mix well) it may take a day or two.

[eLowar]

The cheapest way, and the way a lot of oldschool *nix programs do things by default, is not to use files at all, but simply read from stdin and write to stdout and then use pipes. That way the shell has to do all the dirty work. I'm not sure how well this works on an Amiga, but I do know that the Amiga does support pipes.

Alternatively get your arguments from the optional parameters of main (int main(int argc, char* argv[])), it's really easier than you might think.

Your third option is to look into cross platform GUI toolkits, although that will be much more complex (as you'll have to learn the ways of the particular library) and definitely less portable.

Lastly you could implement any combination of what you have and the above and either decide at compile time what to use (#ifdef __AmigaBuild__ or suchlike) or at runtime (e.g. read from stdin, unless -f parameter is specified, write to stdout unless -o parameter is specified).

Have fun (both drinking and coding).

[Jherek Carnelia]

This *should* be portable, but I have no way to check and not enough knowledge to tell!

Attached is a re-written code with sample Shadow of the Beast copper list disassembly output.

[eLowar]

Two minor warnings:

NewCopDis.c:271: warning: comparison between pointer and integer
NewCopDis.c:232: warning: return type of 'main' is not `int'

The first is due to the fact that you use NULL as an integer (NULL is often defined as (void*)0). fread returns the number of complete objects read, which you should compare with the number 0. NULL is for the null-pointer.

The second is simply because a command line program should return an integer value to the operating system (other languages like ISO C++ won't even give you a choice). Just make the return value int and return 0 in the end.

I'd also just get rid of the getch in the end. It's ugly and it doesn't really serve any purpose if you run the program from a command window.

Other'n that good job, except I can't really test it for lack of a binary example file.

First Edit: Oh and, when you add a return value to main, you could as well change exit(1) to return 1. Just as a side note.

Second Edit: I ought probably to have mentioned what I compiled this with. This is with GCC/MinGW 3.4.2 under Windows.

[Jherek Carnelia]

Thanks for your time elowar.

Just for completeness is a memory dump of Shadow of the Beast's copperlist. Zipped because it has no filetype really.

[eLowar]

Here's the output. Looks good as far as I can tell.

Edit: Maybe not... It doesn't match the one that comes with your code. Well, I have to go now, you figure it out.

[Jherek Carnelia]

The problem is the "endianess" differences between the Amiga and the PC. As you know, PCs store the bytes of a word with the MSB at the highest address and Amigas the opposite way round.

The program now does a check (thanks to a quick google!) to see which way around the host machine stores bytes in a word.

Hopefully this will fix it...

[eLowar]

Yeah I started wondering about that on the way out, but I didn't have time to come back and post. Very good job!

First Edit: Sorry, I spoke too fast, your check does not seem to work, my Windows PC is detected as an Amiga. When I hardcode it to PC the output still differs. Seems to need some more work.

Maybe this was a bad (or very good ) choice for an exercise in portability.

Second Edit: I'm not yet sure what the problem is, but to avoid problems with differently sized file types on different systems/compilers, you might want to have a look at the header stdint.h, which contains (or should contain) system and compiler specific typedefs for exact size datatypes. Just a thought.

Third Edit: The differences are actually minor, so you're probably on the right way (except for the non-working check), e.g.

DC.W $db01,$ff00 ;WAIT: VP=$db (219),HP=$01 (1):VE=$ff (255),HE=$00 (0) (correct) vs.
DC.W $db01,$ff00 ;WAIT: VP=$01 (1),HP=$db (219):VE=$00 (0),HE=$ff (255) (incorrect)

You'll notice that the majority of lines as well as the general structure is identical.

FYI, I also tested it on an x86 Linux system with gcc 3.3.4 now and the check doesn't work there either.