stack smashing dealing with files...

This is a discussion on stack smashing dealing with files... within the C Programming forums, part of the General Programming Boards category; hello everybody! I wanted to post this thread just to ask for your comments about this code, but unfortunately i ...

  1. #1
    Registered User
    Join Date
    Oct 2007
    Posts
    100

    stack smashing dealing with files...

    hello everybody!
    I wanted to post this thread just to ask for your comments about this code, but unfortunately i just realized that i have also a bug in it so i have to make a double question for help and comments...
    The excercise should be (is) very simple: open a file and copy it to another file with the same name followed by ".short" by leaving at most 1 empty line when more than 1 consecutive such lines are met.
    Example:
    [\n]
    hello
    [\n]
    [\n]
    should become the same without the last [\n].

    I wanted to write a program which checked all the bad cases (thing which usually i don't do, above all while writing files.. ) and this is my program:

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    int main (void)
    {
    	FILE *fPtr,*tmpPtr;
    	char filename[]="examXX906.txt";
    
    	if ((fPtr=fopen(filename,"r"))!=NULL)
    	{
    		if ((tmpPtr=fopen("tmpXX906.txt","w+"))!=NULL)
    		{
    			int ch,isempty=1,emptylines=0;
    
    			while ((ch=fgetc(fPtr))!=EOF)
    			{
    				if (ch!='\n')
    				{
    					emptylines=0;
    					isempty=0;
    					if (fputc(ch,tmpPtr)==EOF)
    					{
    						printf("ERROR WRITING THE TEMP FILE\n");
    						exit(1);
    					}
    				}
    				else
    				{
    					if ((isempty==1))
    					{
    						emptylines++;
    						if (emptylines<2)
    						{
    							if (fputc(ch,tmpPtr)==EOF)
    							{
    								printf("ERROR WRITING THE TEMP FILE\n");
    								exit(1);
    							}
    						}
    					}
    					else
    					{
    						isempty=1;
    						if (fputc(ch,tmpPtr)==EOF)
    						{
    							printf("ERROR WRITING THE TEMP FILE\n");
    							exit(1);
    						}
    					}
    				}
    			}
    			if (ferror(fPtr))
    			{
    				printf("ERROR WHILE READING SOURCE FILE\n");
    				exit(1);
    			}
    			else
    			{
    				FILE *targetPtr;
    				strcat(filename,".short");
    
    				if ((targetPtr=fopen(filename,"w+"))!=NULL)
    				{
    					rewind(tmpPtr);
    					while ((ch=fgetc(tmpPtr))!=EOF)
    					{
    						if (fputc(ch, targetPtr)==EOF)
    						{
    							printf("ERROR WRITING THE TARGET FILE\n");
    							exit(1);
    						}
    					}
    
    					if (ferror(tmpPtr))
    					{
    						printf("ERROR WHILE READING TEMP FILE\n");
    						exit(1);
    					}
    
    					fclose(fPtr);
    					fclose(targetPtr);
    					fclose(tmpPtr);
    					remove("tmpXX906.txt");
    				}
    				else
    				{
    					fclose(fPtr);
    					fclose (tmpPtr);
    					remove("tmpXX906.txt");
    					printf("Couldn't create target file\n");
    					exit(1);
    				}
    			}
    		}	
    		else
    		{
    			fclose(fPtr);
    			printf("Couldn't open temp file\n");
    			exit(1);
    		}
    	
    	}
    	else
    	{
    		printf("Couldn't open source file\n");
    		exit(1);
    	}
    
    	return 0;
    }
    it seems to work but i get a very worrying "STACK SMASH DETECTED. Aborted" error....
    what is wrong there?

    and above all, since on saturday i will be asked to write a (maybe) similar program on a sheet of paper, i'd kindly like to ask you hints for writing programs that deal with file by minimizing the possibilities of mistakes (if you forget a ";" you don't pass this exam... )..
    For example, i thought to solve this same excercise by using FGETS() but i think that it's better to have a small set of functions which have not a wide variety of worst-cases while writing on paper, do you agree?

    many thanks for all your help/hints!
    Bye

  2. #2
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Code:
    strcat(filename,".short");
    You haven't got space to extend filename, as you give the compiler the task of figuring out the size when you say "char filename[] = ..." - the compiler will use the assigned string to give the array the size, and when you add another 5 letters onto it, it will overflow it's allocated space.

    As a side note, try to generalize your code to not repeat this sequence:
    Code:
    					fclose(fPtr);
    					fclose (tmpPtr);
    					remove("tmpXX906.txt");
    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  3. #3
    Registered User
    Join Date
    Oct 2007
    Posts
    100
    Quote Originally Posted by matsp View Post
    Code:
    strcat(filename,".short");
    You haven't got space to extend filename, as you give the compiler the task of figuring out the size when you say "char filename[] = ..." - the compiler will use the assigned string to give the array the size, and when you add another 5 letters onto it, it will overflow it's allocated space.

    As a side note, try to generalize your code to not repeat this sequence:
    Code:
    					fclose(fPtr);
    					fclose (tmpPtr);
    					remove("tmpXX906.txt");
    --
    Mats
    i figured this problem and tried to recompile before posting... maybe i recompiled without saving the file because now works..
    anyway, thanks a lot!

  4. #4
    Registered User
    Join Date
    Oct 2007
    Posts
    100
    by the way, i realized now that i should add a control for in the case of a file with an empty line at the end.. it gets added to the file anyway..

  5. #5
    Technical Lead QuantumPete's Avatar
    Join Date
    Aug 2007
    Location
    London, UK
    Posts
    894
    Just something I want to mention. You end up with several levels of indentation in your code. I generally go by the rule that code that's running 90% of the time, should be in the lowest level of indentation. Hence, rather than writing:
    Code:
    rcode = foo();
    if (rcode == 0) {
       rval = bar();
       if (rval == 0) {
           /* Do 
               other 
                   stuff, 
                   introducing 
               more 
               nesting */
       } else {
           printf ("bar returned non-zero!\n");
           return 1;
       }
    } else {
       printf ("foo returned non-zero!\n");
       return 1;
    }
    You should do this:
    Code:
    rcode = foo();
    if (rcode != 0) {
       printf ("foo returned non-zero\n");
       return 1;
    }
    rval = bar ();
    if (rval != 0) {
       printf ("bar returned non-zero\n");
       return 1;
    }
    
    /* Now do all the other stuff */
    In other words, don't enter an if statement because everything is hunky-dory, but rather when some sort of unusual condition occurs (such as non-zero rcodes);

    QuantumPete
    "No-one else has reported this problem, you're either crazy or a liar" - Dogbert Technical Support
    "Have you tried turning it off and on again?" - The IT Crowd

  6. #6
    Captain Crash brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,263
    Quote Originally Posted by QuantumPete View Post
    In other words, don't enter an if statement because everything is hunky-dory, but rather when some sort of unusual condition occurs (such as non-zero rcodes);
    This is more a point of argument than you make it sound. I personally have no problem with early returns, and so I code a lot like your second example. But other respectable people believe that early exits are stylistically poor. These people write code sort of like your first example, where the rule is to keep the returns as low in the function as possible, and to minimize their number as much as possible.

    You might think this would lead to crazy deep indentation levels, and it does. However, most programmers who use this practice take that as an indication that their code needs to be broken into smaller functions. And that's a good thing anyway.

  7. #7
    Technical Lead QuantumPete's Avatar
    Join Date
    Aug 2007
    Location
    London, UK
    Posts
    894
    Quote Originally Posted by brewbuck View Post
    This is more a point of argument than you make it sound.
    I only meant to illustrate that nesting if statements like that can be avoided. I prefer the second version and that's what I would always teach people, but you're right, there's nothing *wrong* with the first version. And I agree that in the end it always boils down to a pointless argument like whether the opening brace should be at the end of the line or on the next one.

    QuantumPete
    "No-one else has reported this problem, you're either crazy or a liar" - Dogbert Technical Support
    "Have you tried turning it off and on again?" - The IT Crowd

  8. #8
    Registered User
    Join Date
    Oct 2007
    Posts
    100
    Hello my friends!
    Thanks again for your hints! now it looks much clearer above all if i have to write it on paper!
    I did another excercise: Read from a file which contains numbers written in ternary, convert them to decimal and copy the result in a second file whose name is the same of the source file with extension ".result".
    I coded with the QuantumPete's 2nd indentation and here is the result.
    The only thing which is still not really clear to me is about the way to append the ".result" extension to the filename.
    The program run 2 or 3 times and then i got the error i report below (which looks like a linking problem?). What did i wrong? thanks again!

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <math.h>
    
    unsigned int ternary2dec (unsigned int n);
    int fileisempty(FILE *f);
    
    int main(int argc, char *argv[])
    {
    
    	if (argc!=2)
    	{
    		printf("USAGE: ./esameX307 sourcefilename\n");
    		exit(1);
    	}
    
    	FILE *srcPtr,*tgtPtr;
    	if ((srcPtr=fopen(argv[1],"r"))==NULL)
    	{
    		printf ("Couldn't open file %s\n",argv[1]);
    		exit(1);
    	}
    
    	if (fileisempty(srcPtr)==1)
    	{
    		printf("Source file is empty!\n");
    		exit(1);
    	}
    
    	if (fileisempty(srcPtr)==2)
    	{
    		printf("There was a proble while reading source file!\n");
    		exit(1);
    	}
    	
    	size_t tgtfilenamesize=sizeof(argv[1]);
    	tgtfilenamesize+=8; //.result\0
    	char *tgtFN = (char *) malloc (tgtfilenamesize*sizeof(char));
    	
    	if (tgtFN==NULL)
    	{
    		printf("Couldn't get memory for the target filename\n");
    		exit(1);
    	}
    	
    	strcpy(tgtFN,argv[1]);
    	strcat(tgtFN,".result");
    	puts(tgtFN);
    	if ((tgtPtr=fopen(tgtFN,"w+"))==NULL)
    	{
    		printf ("Couldn't open file %s\n",argv[1]);
    		exit(1);
    	}
    	
    	unsigned int sum=0;
    	unsigned int num;
    
    	while (fscanf(srcPtr,"%u",&num)!=EOF)
    	{
    		sum+=ternary2dec(num);
    	}
    	printf("SUM %d\n",sum);
    	fprintf(tgtPtr,"RESULT: %u",sum);
    	fclose(srcPtr);
    	fclose(tgtPtr);
    	return 0;
    
    }
    
    unsigned int ternary2dec (unsigned int n)
    {
    	unsigned int ret=0;
    	int i=0;
    	while (n>0)
    	{
    		ret += pow(3,i)*(n%10);
    		i++;
    		n/=10;
    		
    	}
    	return ret;
    
    }
    
    int fileisempty(FILE *f)
    {
    	//1: file empty
    	//2: other errors
    	//0: file not empty
    	rewind(f);
    	int ch;
    	ch = fgetc(f);
    	if (ch==EOF)
    	{
    		if (feof(f))
    		{
    			printf("FILE IS EMPTY\n");
    			rewind(f);
    			return 1;
    		}
    		else
    		{
    			printf("Other errors reading source file\n");
    			rewind(f);
    			return 2;
    		}
    	}
    	rewind(f);
    	return 0;
    }
    Code:
    ./esameX307 esameX307.txt
    esameX307.txt.result
    SUM 134
    *** glibc detected *** ./esameX307: free(): invalid next size (normal): 0x0804a180 ***
    ======= Backtrace: =========
    /lib/tls/i686/cmov/libc.so.6[0xb7de1d65]
    /lib/tls/i686/cmov/libc.so.6(cfree+0x90)[0xb7de5800]
    /lib/tls/i686/cmov/libc.so.6(fclose+0x134)[0xb7dd06f4]
    ./esameX307[0x8048899]
    /lib/tls/i686/cmov/libc.so.6(__libc_start_main+0xe0)[0xb7d8e050]
    ./esameX307[0x8048621]
    ======= Memory map: ========
    08048000-08049000 r-xp 00000000 08:02 3409197    /home/federico/universitÓ/programmazione C/esercizi/esameX307
    08049000-0804a000 rw-p 00000000 08:02 3409197    /home/federico/universitÓ/programmazione C/esercizi/esameX307
    0804a000-0806b000 rw-p 0804a000 00:00 0          [heap]
    b7c00000-b7c21000 rw-p b7c00000 00:00 0
    b7c21000-b7d00000 ---p b7c21000 00:00 0
    b7d77000-b7d78000 rw-p b7d77000 00:00 0
    b7d78000-b7ebc000 r-xp 00000000 08:02 1049547    /lib/tls/i686/cmov/libc-2.6.1.so
    b7ebc000-b7ebd000 r--p 00143000 08:02 1049547    /lib/tls/i686/cmov/libc-2.6.1.so
    b7ebd000-b7ebf000 rw-p 00144000 08:02 1049547    /lib/tls/i686/cmov/libc-2.6.1.so
    b7ebf000-b7ec2000 rw-p b7ebf000 00:00 0
    b7ec2000-b7ee5000 r-xp 00000000 08:02 1049551    /lib/tls/i686/cmov/libm-2.6.1.so
    b7ee5000-b7ee7000 rw-p 00023000 08:02 1049551    /lib/tls/i686/cmov/libm-2.6.1.so
    b7ee8000-b7ef2000 r-xp 00000000 08:02 1015875    /lib/libgcc_s.so.1
    b7ef2000-b7ef3000 rw-p 0000a000 08:02 1015875    /lib/libgcc_s.so.1
    b7ef3000-b7ef4000 rw-p b7ef3000 00:00 0
    b7ef5000-b7ef7000 rw-p b7ef5000 00:00 0
    b7ef7000-b7f11000 r-xp 00000000 08:02 1015821    /lib/ld-2.6.1.so
    b7f11000-b7f13000 rw-p 00019000 08:02 1015821    /lib/ld-2.6.1.so
    bf98f000-bf9a5000 rw-p bf98f000 00:00 0          [stack]
    ffffe000-fffff000 r-xp 00000000 00:00 0          [vdso]
    Aborted (core dumped)

  9. #9
    Just Lurking Dave_Sinkula's Avatar
    Join Date
    Oct 2002
    Posts
    5,006
    Code:
    	size_t tgtfilenamesize=sizeof(argv[1]);
    You probably want
    Code:
    	size_t tgtfilenamesize=strlen(argv[1]);
    7. It is easier to write an incorrect program than understand a correct one.
    40. There are two ways to write error-free programs; only the third one works.*

  10. #10
    CSharpener vart's Avatar
    Join Date
    Oct 2006
    Location
    Rishon LeZion, Israel
    Posts
    6,484
    Code:
    if (fileisempty(srcPtr)==1)
    	{
    		printf("Source file is empty!\n");
    		exit(1);
    	}
    
    	if (fileisempty(srcPtr)==2)
    	{
    		printf("There was a proble while reading source file!\n");
    		exit(1);
    	}
    instead of calling same function twice in a row - call it once, store the result in some temp var and check the variable value to be 1 or 2
    The first 90% of a project takes 90% of the time,
    the last 10% takes the other 90% of the time.

  11. #11
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,761
    > size_t tgtfilenamesize=sizeof(argv[1]);
    Maybe you mean strlen, not sizeof
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  12. #12
    Registered User
    Join Date
    Oct 2007
    Posts
    100
    Quote Originally Posted by Dave_Sinkula View Post
    Code:
    	size_t tgtfilenamesize=sizeof(argv[1]);
    You probably want
    Code:
    	size_t tgtfilenamesize=strlen(argv[1]);
    ops... i really hoped it was not a conceptual mistake!
    THX!

  13. #13
    Captain Crash brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,263
    Quote Originally Posted by smoking81 View Post
    ops... i really hoped it was not a conceptual mistake!
    THX!
    Well, confusing sizeof() and strlen() could be viewed as a conceptual mistake.

  14. #14
    Registered User
    Join Date
    Oct 2007
    Posts
    100
    Quote Originally Posted by brewbuck View Post
    Well, confusing sizeof() and strlen() could be viewed as a conceptual mistake.
    i know it could be conceptual, fortunately i had strlen() in my mind..

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Smashing The Stack
    By 0624167 in forum Linux Programming
    Replies: 1
    Last Post: 03-03-2009, 02:09 PM
  2. Fixing my program
    By Mcwaffle in forum C Programming
    Replies: 5
    Last Post: 11-05-2008, 03:55 AM
  3. Stringy Sums
    By bumfluff in forum C++ Programming
    Replies: 14
    Last Post: 05-15-2006, 02:52 AM
  4. Folding@Home Cboard team?
    By jverkoey in forum A Brief History of Cprogramming.com
    Replies: 398
    Last Post: 10-11-2005, 09:44 AM
  5. error trying to compile stack program
    By KristTlove in forum C++ Programming
    Replies: 2
    Last Post: 11-03-2003, 06:27 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21