2

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); } 
3
  • 5
    I assume this is some old code you found somewhere? It is very old, and uses the K&R, pre-ANSI, variant for defining function arguments. As for the error, the malloc function is declared in the <stdlib.h> header file. Code using malloc should include that header file and not declare it itself. Which is done (still in pre-ANSI way) in the emalloc function. Commented Dec 13, 2017 at 13:27
  • 4
    My advice for you: Throw this code away. Whatever it do I'm certain there have been better alternatives developed the last 30 or so years. Commented Dec 13, 2017 at 13:29
  • Note that testing for succesful allocation is very poorly done here. It simply prints out of memory to console, but then continues like everything went fine, resulting in undefined behaviour. Commented Dec 13, 2017 at 13:40

4 Answers 4

3

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.

Sign up to request clarification or add additional context in comments.

3 Comments

Would not replacing rfixpro (ns, pH... with int rfixpro ( int ns, double pH, double beta, float pKint[], int ida[], float g[], double cutoff, float cs[], float *tot)? Replace float objects with double ones - yet leave pointers to match
@chux For rfixpro(), yes, that should work. But given the calls to functions such as tc() and pfix() in the posted code, I suspect that there are a lot of functions in the entire body of code the questioner is working on, all of them in K&R form. I'd bet a large sum against all of those functions being convertible to standard-compliant form, having all calls done without a prototype, and not breaking anything.
Agree, My experience supports that it (prototyping) is doable - yet quite error prone.
2

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

4 Comments

Thanks for your comments! Removing the char * emallo part gives me the following: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] fprot = (int *) emalloc ((unsigned) ns * sizeof (int));
@AndreasTosstorff If you removed line char * emallo, you did it wrong. This answer refers to line char *p, malloc();.
I removed this: char * emalloc (n) unsigned n; { char *p, malloc(); p = malloc(n); if (p == NULL) printf ("out of memory\n"); return p; } and replaced emalloc with malloc in the fprot =... line . Does this make sense?
@AndreasTosstorff I'm talking about a single line, the line where it says , malloc(); inside the function. If you remove the entire emalloc() function the code won't build. Also, try to build with ancient language spec in mind, if your compiler supports that.
1

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.

1 Comment

1 of 2 answers to correctly explain the error message.
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! } 

6 Comments

You can (at least you could in the past...) do it, and it's not weird. It's merely old-style K&R C code from the time before even ANSI C. Unless you really understand what you're doing, do not go blindly changing K&R-style C functions into current standard C. You will break things badly.
@AndrewHenle: True, I just checked that. But to be honest, I have never seen such a usage till date in a live project. Although, I do agree that I am too young to comment on that, 5 years in C development is dirt cheap experience.
5 years in C development is dirt cheap experience. No, 5 years is more than enough to learn where to look when you find things you don't understand, which is really the important part of experience with any programming language.
@AndrewHenle: Again, I have to agree :-) Its just that I had never seen such a code and haven't tried a many codes from K&R C book.
It's certainly ancient. And it looks broken, too. char *p, malloc(); is really suspect as it seems to declare that malloc() returns char, which should break things. I'm guessing the compiler used on that code wasn't very thorough and didn't actually truncate the return from malloc() down to a char. It seems that compiler just used whatever happened to be in the return value register without the proper type conversion the char malloc() prototype specifies.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.