What is wrong with this function?

This is a discussion on What is wrong with this function? within the C Programming forums, part of the General Programming Boards category; Code: int funkcija(int niz[N]){ int i,*n,j=0; n=malloc(sizeof(niz)); for(i=0;i<N;i++){ n[i]=niz[i]; } return n; }...

  1. #1
    Sometimes so stupid... shardin's Avatar
    Join Date
    Jul 2007
    Location
    Dalmatia/CRO
    Posts
    78

    What is wrong with this function?

    Code:
    int funkcija(int niz[N]){
    
    	int i,*n,j=0;
    		
    	n=malloc(sizeof(niz));
    	
    	for(i=0;i<N;i++){
    		n[i]=niz[i];
    						
    	}
    		return n;
    	}
    ...and aprentice shall become master...or not...

    "Never let your sense of moral prevent you from doing what is right!" Salvor Hardin, mayor of Terminus

  2. #2
    ZuK
    ZuK is offline
    Registered User
    Join Date
    Aug 2005
    Location
    Austria
    Posts
    1,990
    The return value of funkcija is int - you try to return a pointer to int.
    sizeof(niz) will give you the size of a pointer - you will write out of bounds if N > sizeof(int*)
    Kurt

  3. #3
    Sometimes so stupid... shardin's Avatar
    Join Date
    Jul 2007
    Location
    Dalmatia/CRO
    Posts
    78
    It is not working, why? I dont know! And I would like to know, so please...

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #define N 3
    int main()
    
    {
    	int sranje,el;
    	int niz[N];
    		int i;
    
    	printf("Unesite broj elemenata  niza: ");
    	scanf("%d",&el);
    
    		for(i=0;i<el;i++){
    		printf("%d. ",i+1);
    		scanf("%d", &niz[i]);}
    
    	for(i=0;i<el;i++){
    	printf("%d",funkcija(el,niz));
    	}
    
    	scanf("%d", &sranje);
    }
    
    int funkcija(int el,int *p1){
    
    	int i,*n;
    		
    	n=(int *)malloc(sizeof(int)*(sizeof(p1)));
    	
    	for(i=0;i<el;i++){
    		n[i]=p1[i];
    	}
    		return n;
    	}
    ...and aprentice shall become master...or not...

    "Never let your sense of moral prevent you from doing what is right!" Salvor Hardin, mayor of Terminus

  4. #4
    ZuK
    ZuK is offline
    Registered User
    Join Date
    Aug 2005
    Location
    Austria
    Posts
    1,990
    Quote Originally Posted by shardin View Post
    It is not working, why?
    Please define "not working". I's not clear at all what you are trying to do.

    There are still the same problems.

    The return value of funkcija is int - you try to return a pointer to int.
    The size that you pass to malloc doesn't make sense, you're allocationg sizeof a pointer * sizeof an int bytes.
    guess you want
    Code:
    n=malloc(sizeof(int)*el);
    and you never free the allocated space.
    Kurt

  5. #5
    Sometimes so stupid... shardin's Avatar
    Join Date
    Jul 2007
    Location
    Dalmatia/CRO
    Posts
    78
    Now its like this...
    Code:
    int funkcija(int el,int *p1){
    
    	int i,*n;
    		
    	n=malloc(sizeof(int)*el);
    	
    	for(i=0;i<el;i++){
    		n[i]=p1[i];
    	}
    		return n;
    	}
    It returns a bunch of numbers, probably some chunk of memory, mem. address or something,
    numbers are always the same. Why do I have to free memory after, with strings this works without problem!
    ...and aprentice shall become master...or not...

    "Never let your sense of moral prevent you from doing what is right!" Salvor Hardin, mayor of Terminus

  6. #6
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,452
    Is your compiler issuing any warnings at all, or are you just ignoring them?
    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.

  7. #7
    Sometimes so stupid... shardin's Avatar
    Join Date
    Jul 2007
    Location
    Dalmatia/CRO
    Posts
    78
    no warnings at all, nothing. result is just wrong.
    ...and aprentice shall become master...or not...

    "Never let your sense of moral prevent you from doing what is right!" Salvor Hardin, mayor of Terminus

  8. #8
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,452
    Your code, compiled
    Code:
    $ gcc -W -Wall -ansi -pedantic foo.c
    foo.c: In function `main':
    foo.c:19: warning: implicit declaration of function `funkcija'
    foo.c:23: warning: control reaches end of non-void function
    foo.c: In function `funkcija':
    foo.c:34: warning: return makes integer from pointer without a cast
    I would suggest a better compiler, or at least turning up the warning level as much as possible on the one you do have.
    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.

  9. #9
    Sometimes so stupid... shardin's Avatar
    Join Date
    Jul 2007
    Location
    Dalmatia/CRO
    Posts
    78
    I am using Microsoft Visual C++ 6.0. I see no option for level of warnings.
    ...and aprentice shall become master...or not...

    "Never let your sense of moral prevent you from doing what is right!" Salvor Hardin, mayor of Terminus

  10. #10
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,452
    Project->settings

    Although you can set warning level to 4, I find that many of Microsoft's headers are broken at that level.

    The later versions (VS2003 and beyond) seem much more capable of compiling code at W4 level without too much collateral damage from the system header files.

    Or you could just get MinGW / Dev-C++ as well and try to compile the same code with that compiler as well.

    It may seem like extra work (initially), but if you can get the same code to compile with 2 different compilers, and produce the same results, then you're probably doing something right.
    Attached Images Attached Images  
    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.

  11. #11
    Sometimes so stupid... shardin's Avatar
    Join Date
    Jul 2007
    Location
    Dalmatia/CRO
    Posts
    78
    yes in Dev, it gives me this warning,
    foo.c: In function `funkcija':
    foo.c:34: warning: return makes integer from pointer without a cast
    ,
    and in other task that iam doing also. The problem is that iam doing something wrong when trying to return a *pointer on a int. Other way it works. Well il try to find what's the problem and hopefully will discover solution!
    ...and aprentice shall become master...or not...

    "Never let your sense of moral prevent you from doing what is right!" Salvor Hardin, mayor of Terminus

  12. #12
    Technical Lead QuantumPete's Avatar
    Join Date
    Aug 2007
    Location
    London, UK
    Posts
    894
    I think you need to change your return type to int*. At the moment you're returning 'n' which is a memory address and it's put into an int, which is why you get random numbers. Change the return type of the function and then dereference on the other side, free the allocated memory and hey presto, it should work.

    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

  13. #13
    Sometimes so stupid... shardin's Avatar
    Join Date
    Jul 2007
    Location
    Dalmatia/CRO
    Posts
    78
    It is working now! Hura!!!!

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #define N 30
    int *funkcija(int *, int);
    int main(int argc, char *argv[])
    {
    	int el;
    	int niz[N];
    		int i,*pp;
    
    	printf("Unesite broj elemenata  niza: ");
    	scanf("&#37;d",&el);
    
    		for(i=0;i<el;i++){
    		printf("%d. ",i+1);
    		scanf("%d", &niz[i]);}
    	
    	pp=funkcija(niz,el);
    	for(i=0;i<el;i++){
                          printf("%d", pp[i]);}
    	  
      system("PAUSE");	
      return 0;
    }
    
    int *funkcija(int *p1,int el){
    
    	int i,*n;
    		
    	n=malloc(sizeof(int)*el);
    	
    	for(i=0;i<el;i++){
    		n[i]=p1[i];
    	}
    			return n;
    	}
    ...and aprentice shall become master...or not...

    "Never let your sense of moral prevent you from doing what is right!" Salvor Hardin, mayor of Terminus

  14. #14
    Technical Lead QuantumPete's Avatar
    Join Date
    Aug 2007
    Location
    London, UK
    Posts
    894
    You should free the memory pointed to by pp. I know it's not strictly necessary, since the program exits, but it's still good practice.

    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

  15. #15
    Code Goddess Prelude's Avatar
    Join Date
    Sep 2001
    Posts
    9,796
    >I know it's not strictly necessary, since the program exits, but it's still good practice.
    How do you know it's not strictly necessary? You're assuming that the host will release memory allocated by malloc, and the standard makes no such guarantee. Always free the memory you allocated.

    >int *funkcija(int *, int);
    I'm a huge fan of always providing parameter names. It makes it vastly simpler to understand what's going on when you only have a declaration to work with (such as in a header) or the definition is buried somewhere deep in the bowels of a monolithic source file. It also makes it easier to match the declaration with the definition because they're identical:
    Code:
    int *funkcija(int *p1,int el);
    
    /* ... */
    
    int *funkcija(int *p1,int el){
      int i,*n;
    		
      n=malloc(sizeof(int)*el);
    	
      for(i=0;i<el;i++){
        n[i]=p1[i];
      }
    
      return n;
    }
    >int main(int argc, char *argv[])
    While I appreciate your attempt to use a standard definition of main, there's really no need to include the parameters if you don't use them.

    >scanf("%d",&el);
    Always check the return value of input functions. scanf returns the number of items that were successfully read, so you can do this:
    Code:
    if ( scanf ( "%d", &el ) != 1 ) {
      fputs ( "Invalid input\n", stderr );
      return EXIT_FAILURE;
    }
    >system("PAUSE");
    This is probably the worst possible way of keeping your program running. It has serious portability and security risks, so you should be using something more like this:
    Code:
    #include <stdio.h>
    
    void jsw_flush ( FILE *in, int delim )
    {
      int ch;
    
      do
        ch = fgetc ( in );
      while ( ch != delim && ch != EOF );
    }
    
    int main ( void )
    {
      /* ... */
    
      jsw_flush ( stdin, '\n' );
      fputs ( "Press Enter to continue . . .", stdout );
      fflush ( stdout );
      getchar();
    }
    >int i,*n;
    It's generally bad practice to declare multiple variables on one line, especially when one or more of those variables are pointers or have initialization values. It's very easy to make a mistake and difficult to document using comments.

    >n=malloc(sizeof(int)*el);
    The convention for malloc is:
    Code:
    n = malloc ( el * sizeof *n );
    This way you don't have to worry about matching up the type used in the sizeof expression with the type of the pointer being assigned to. Also, n is an extremely poor name for a pointer variable. Usually n is used to designate an integral value, such as the size of an array.
    My best code is written with the delete key.

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. In over my head
    By Shelnutt2 in forum C Programming
    Replies: 1
    Last Post: 07-08-2008, 06:54 PM
  2. Game Pointer Trouble?
    By Drahcir in forum C Programming
    Replies: 8
    Last Post: 02-04-2006, 01:53 AM
  3. Replies: 3
    Last Post: 03-04-2005, 01:46 PM
  4. structure vs class
    By sana in forum C++ Programming
    Replies: 13
    Last Post: 12-02-2002, 06:18 AM
  5. Contest Results - May 27, 2002
    By ygfperson in forum A Brief History of Cprogramming.com
    Replies: 18
    Last Post: 06-18-2002, 01:27 PM

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