Thread: 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;
	}

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;
	}

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!

Is your compiler issuing any warnings at all, or are you just ignoring them?

no warnings at all, nothing. result is just wrong.

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.

I am using Microsoft Visual C++ 6.0. I see no option for level of warnings.

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.

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!

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

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;
	}

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

>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.