error: conflicting types for ‘malloc’

Question

I am getting the following error and warning for a script:

rfixpro.c:7:14: error: conflicting types for ‘malloc’
     char *p, malloc();
              ^
rfixpro.c:9:7: warning: assignment makes pointer from integer without a cast [enabled by default]
     p = malloc(n);

This is part of a program I am trying to install by using make. I never used C++ befor so please excuse my ignorance. I tried to run the script without the 'include' lines, but this produces other warnings. I therefore also post the entire script.

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

char * emalloc (n)
unsigned n;
{
    char *p, malloc();

    p = malloc(n);
    if (p == NULL)
    printf ("out of memory\n");
    return p;
}


int rfixpro (ns, pH, beta, pKint, ida, g, cutoff, cs, tot)
     int ns;        /* Number of titratable sites           */
     float pH;
     float beta;        /* 1 / kT */
     float pKint[]; /* Intrinsic pK of sites (input)        */
     int ida[];         /* ida=1 for cation 2 for anion  (input)    */
     float g[];     /* site - site interaction matrix (input)   */
     float cutoff;       /* Cutoff value for fixing sites (input) */
     float cs[];        /* degree of protonation of sites (output) */
     float *tot;        /* total protonation (output) */
{
  int nsr;  /* Number of sites in reduced set   */
  int *fprot;   /* = 1 or 0 if fixed prot or deprot = -1 if unfixed  */
  int *rmap;    /* rmap[i] = full set index of reduced set site i  */
  int *fmap;    /* fmap[i] = reduced set index of full set site i  */
        /* fmap[i] = -1 if i not in reduced set */
  float *pKintr;    /* Intr. pKs for reduced set    */
  int *idar;    /* Reduced form of ida  */
  float *gr;    /* Reduced form of g    */
  float *csr;   /* Reduced form of cs */
  int i;

  fprot = (int *) emalloc ((unsigned) ns * sizeof (int));
  rmap = (int *) emalloc ((unsigned) ns * sizeof (int));
  fmap = (int *) emalloc ((unsigned) ns * sizeof (int));
  pKintr = (float *) emalloc ((unsigned) ns * sizeof (float));
  idar = (int *) emalloc ((unsigned) ns * sizeof (int));
  gr = (float *) emalloc ((unsigned) ns*ns * sizeof (float));
  csr = (float *) emalloc ((unsigned) ns * sizeof (float));

  pfix (ns, pH, beta, pKint, ida, g, cutoff,
    &nsr, fprot, rmap, fmap, pKintr, idar, gr);
  tc (nsr, pKintr, idar, gr, pH, beta, csr);
  *tot = 0.0;
  for (i=0; i<ns; ++i)
    {
      if (fmap[i] != -1)
    cs[i] = csr[fmap[i]];
      else 
    cs[i] = (float) fprot[i];
      *tot += cs[i];
    }

  free ( (char *) fprot);
  free ( (char *) rmap);
  free ( (char *) fmap);
  free ( (char *) pKintr);
  free ( (char *) idar);
  free ( (char *) gr);
  free ( (char *) csr);

  return (nsr);
}

Answer 1

This is absurd.

char *p, malloc();

You can not do this.

Also,

char * emalloc (n)
unsigned n;
{

    //some code

is valid, oldschool, and might appear weird to most of the younger bunch of programmers around.

malloc allocates requested bytes of memory (if available), and return a void pointer pointing to the address where it is allocated.

This is the way it should be:

void* emalloc(unsigned int n)
{
    void *p = NULL;
    p = malloc(n);

    if(p == NULL)
    {
        printf("No memory");
        //Will return NULL anyway!!!    
    }

    return p;
}

Additionally, before all these pointers are accessed, you'd have to check if there surely not NULL.

fprot = (int *) emalloc ((unsigned) ns * sizeof (int));
rmap = (int *) emalloc ((unsigned) ns * sizeof (int));
fmap = (int *) emalloc ((unsigned) ns * sizeof (int));
pKintr = (float *) emalloc ((unsigned) ns * sizeof (float));
idar = (int *) emalloc ((unsigned) ns * sizeof (int));
gr = (float *) emalloc ((unsigned) ns*ns * sizeof (float));
csr = (float *) emalloc ((unsigned) ns * sizeof (float));

Like,

fprot = (int *) emalloc ((unsigned) ns * sizeof (int));
if(fproat == NULL)
{
    //Handle the error here!
}

Plus,

I see all the free() calls there, which should only be applied if malloc() didn't return NULL.

But, now, do you see redundant code here? Why do you need an additional function call emalloc() if you are doing the lame thing as calling a malloc() and returning the pointer, you could do it here itself.

fprot = malloc ((unsigned) ns * sizeof (int)); //Don't cast the malloc!
if(fproat == NULL)
{
    //Handle the error here!
}

Answer 2

It's just old-style K&R C code.

char * emalloc (n)
unsigned n;
{
    char *p, malloc();

    p = malloc(n);
    if (p == NULL)
    printf ("out of memory\n");
    return p;
}

Is just a wrapper around malloc(). I think this

char *p, malloc();

is supposed to be a malloc() prototype along with char *p. Although it seems to declare that malloc() returns a char, because

char *p, malloc();

is the equivalent of

char *p;
char malloc();

That's one really good reason to never define more than one variable per line. You're not saving lines of code in a textbook. And it looks like a bad mistake, which is why I just say that I think it's supposed to be a prototype for malloc() - it shouldn't have worked at all if it truncated a pointer down to a char as done on any hardware and C compiler I've ever used.

And it's that char malloc() prototype that causes you to get the error: conflicting types for ‘malloc’ error.

Note that you can't simply go around and replace

int rfixpro (ns, pH, beta, pKint, ida, g, cutoff, cs, tot)
     int ns;        /* Number of titratable sites           */
     float pH;
     float beta;        /* 1 / kT */
     float pKint[]; /* Intrinsic pK of sites (input)        */
     int ida[];         /* ida=1 for cation 2 for anion  (input)    */
     float g[];     /* site - site interaction matrix (input)   */
     float cutoff;       /* Cutoff value for fixing sites (input) */
     float cs[];        /* degree of protonation of sites (output) */
     float *tot;        /* total protonation (output) */
{
    ...

with

int rfixpro ( int ns, float pH, float beta, float pKint[],
    int ida[], float g[], float cutoff, float cs[], float *tot)
{

as that will break things unless you also supply every single call for that function with a proper declaration prior to its use. I seem to remember a long description in one of Peter van der Linden's books for what havoc can come about from doing that.

Answer 3

The problem is that this code is old, pre-standard "K&R C" (K&R 1st edition). You cannot compile it on standard compliant compiler.

In the ancient days when dinosaurs walked the earth, malloc had the form char* malloc ();. This was before C was standardized and before void pointers were even invented.

The sloppy line char *p, malloc(); declares a character pointer p, and then it incorrectly declares malloc as char malloc(). This is likely a bug in the original code.

Now when you show this code to a modern C compiler, with the standard declaration void* malloc(size_t size); in stdlib.h, you get conflicting types. Notably, ancient K&R C didn't require a return type or a parameter list, it just "made up" some types if you didn't specify them explicitly.

My recommendation is that you throw this source code in the garbage bin. It is ancient and contains absolutely nothing of value.

Answer 4

That makes no sense.

The code looks almost garbled, it should just be

void * const p = malloc(n);

no point in brokenly re-declaring malloc(). Note that <stdlib.h> is being included, so a proper prototype is already present.

Also its return type in modern C should be the same as malloc()'s, i.e. void *.