2

I've just started learning C (coming from a C# background.) For my first program I decided to create a program to calculate factors. I need to pass a pointer in to a function and then update the corresponding variable.

I get the error 'Conflicting types for findFactors', I think that this is because I have not shown that I wish to pass a pointer as an argument when I declare the findFactors function. Any help would be greatly appreciated!

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

int *findFactors(int, int);

int main (int argc, const char * argv[]) 
{
    int numToFind;

do {
    printf("Enter a number to find the factors of: ");
    scanf("%d", &numToFind);    
} while (numToFind > 100);

    int factorCount;
findFactors(numToFind, &factorCount);

return 0;
}

int *findFactors(int input, int *numberOfFactors)
{
int *results = malloc(input);
int count = 0;
for (int counter = 2; counter < input; counter++) {
    if (input % counter == 0){
        results[count] = counter;
        count++;
        printf("%d is factor number %d\n", counter, count);
    }
}

return results;
}
unwind
  • 391,730
  • 64
  • 469
  • 606
JoeS88
  • 21
  • 1

5 Answers5

3

Change the declaration to match the definition:

int *findFactors(int, int *);
unwind
  • 391,730
  • 64
  • 469
  • 606
2

I apologise for adding yet another answer but I don't think anyone has covered every point that needs to be covered in your question.

1) Whenever you use malloc() to dynamically allocate some memory, you must also free() it when you're done. The operating system will, usually, tidy up after you, but consider that you have a process during your executable that uses some memory. When said process is done, if you free() that memory your process has more memory available. It's about efficiency.

To use free correctly:

int* somememory = malloc(sizeyouwant * sizeof(int));
// do something
free(somememory);

Easy.

2) Whenever you use malloc, as others have noted, the actual allocation is in bytes so you must do malloc(numofelements*sizeof(type));. There is another, less widely used, function called calloc that looks like this calloc(num, sizeof(type)); which is possibly easier to understand. calloc also initialises your memory to zero.

3) You do not need to cast the return type of malloc. I know a lot of programming books suggest you do and C++ mandates that you must (but in C++ you should be using new/delete). See this question.

4) Your function signature was indeed incorrect - function signatures must match their functions.

5) On returning pointers from functions, it is something I discourage but it isn't wrong per se. Two points to mention: always keep 1) in mind. I asked exactly what the problem was and it basically comes down to keeping track of those free() calls. As a more advanced user, there's also the allocator type to worry about.

Another point here, consider this function:

int* badfunction()
{
    int x = 42;
    int *y = &x;
    return y;
}

This is bad, bad, bad. What happens here is that we create and return a pointer to x which only exists as long as you are in badfunction. When you return, you have an address to a variable that no longer exists because x is typically created on the stack. You'll learn more about that over time; for now, just think that the variable doesn't exist beyond its function.

Note that int* y = malloc(... is a different case - that memory is created on the heap because of the malloc and therefore survives the end of said function.

What would I recommend as a function signature? I would actually go with shybovycha's function with a slight modification:

int findFactors(int* factors, const int N);

My changes are just personal preference. I use const so that I know something is part of the input of a function. It isn't strictly necessary with just an int, but if you're passing in pointers, remember the source memory can be modified unless you use const before it, whereon your compiler should warn you if you try to modify it. So its just habit in this case.

Second change is that I prefer output parameters on the left because I always think that way around, i.e. output = func(input).

Why can you modify function arguments when a pointer is used? Because you've passed a pointer to a variable. This is just a memory address - when we "dereference" it (access the value at that address) we can modify it. Technically speaking C is strictly pass by value. Pointers are themselves variables containing memory addresses and the contents of those variables are copied to your function. So a normal variable (say int) is just a copy of whatever you passed in. int* factors is a copy of the address in the pointer variable you pass in. By design, both the original and this copy point to the same memory, so when we dereference them we can edit that memory in both the caller and the original function.

I hope that clears a few things up.

Community
  • 1
  • 1
  • + for mentioning free(). As the original poster is a C# programmer, they are likely clueless about manual memory cleanup; they are used to garbage collection. – Lundin Jan 31 '11 at 13:45
  • +1 from an 8 yr realtime systems developer is quite a compliment. Thanks. I can't believe nobody else spotted it or felt it wasn't worthy of mention. –  Jan 31 '11 at 14:31
1

EDIT: no reference in C (C++ feature)

Don't forget to modify numberOfFactors in the method (or remove this parameter if not useful). The signature at the beginning of your file must also match the signature of the implementation at the end (that's the error you receive).

Finally, your malloc for results is not correct. You need to do this:

int *results = malloc(input * sizeof(int));
Benoit Thiery
  • 6,325
  • 4
  • 22
  • 28
0
int* ip   <- pointer to a an int
int** ipp <- pointer to a pointer to an int.
RobLucas
  • 435
  • 4
  • 9
0

int *findFactors(int, int); line says you wanna return pointer from this function (it's better to use asteriks closer to the type name: int* moo(); - this prevents misunderstandings i think).

If you wanna dynamically change function argument (which is better way than just return pointer), you should just use argument as if you have this variable already.

And the last your mistake: malloc(X) allocates X bytes, so if you want to allocate memory for some array, you should use malloc(N * sizeof(T));, where N is the size of your array and T is its type. E.g.: if you wanna have int *a, you should do this: int *a = (int*) malloc(10 * sizeof(int));.

And now here's your code, fixed (as for me):

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

int findFactors(int, int*);

int main(int argc, char **argv) 
{
  int numToFind, *factors = 0, cnt = 0;

  do 
  {
    printf("Enter a number to find the factors of: ");
    scanf("%d", &numToFind);    
  } while (numToFind > 100);

  cnt = findFactors(numToFind, factors);

  printf("%d has %d factors.\n", numToFind, cnt);

  return 0;
}

int findFactors(int N, int* factors)
{
  if (!factors)
    factors = (int*) malloc(N * sizeof(int));

  int count = 0;

  for (int i = 2; i < N; i++) 
  {
    if (N % i == 0)
    {
      factors[count++] = i;
      printf("%d is factor number #%d\n", i, count);
    }
  }

  return count;
}

Note: do not forget to initialize your pointers any time (as i did). If you do want to call function, passing a pointer as its argument, you must be sure it has value of 0 at least before function call. Otherwise you will get run-time error.

shybovycha
  • 11,556
  • 6
  • 52
  • 82
  • Memory returned by `malloc` is not zero initialized. You should add a `memset(factors, 0, sizeof(int) * N);` otherwise the values will be undefined. – Sylvain Defresne Jan 31 '11 at 11:16