Malloc doubt

[guillermoandres]

Hello everyone.

I have a little doubt about freeing allocated memory. I'm using this two functions.

gchar* status_code (gchar *status)
{
gchar* data = "";

sip_status ++;
while (*status != CR && *(status+1) != LF)
{
data = concatenar_char(data,status);
status ++;
}

return data;
}/*status_code*/

gchar * concatenar_char (gchar* str1, gchar* str2)
{
gchar *result;
if ( str2 != NULL )
{
result = (gchar *)malloc(strlen(str1) + sizeof(gchar) + 1);
if ( resultado != NULL )
{
strcpy(result, str1);
strncat(result, str2, 1);
}
}
return result;
}/*concatenar_char*/

In the second function I cant free result before return it. So where can I free the allocated memory (result) without getting an error from the compiler??

Is there a better way to do this?

Thanks in advance.

 

[cpjust]

First, what's a gchar?

Second, don't cast malloc(). See:

http://faq.cprogramming.com/cgi-bin/smartfaq.cgi?answer=1047673478&id=1043284351

Third:

data = concatenar_char(data,status);

You just lost your data pointer since the data pointer returned is not the data pointer being passed in.

Instead of allocating the memory inside your concatenar_char() function, why not pass str1 and the size of str1 to the function, then leave it up to the caller to allocate enough memory. Your function could return an int instead (0 if successful, a positive number which indicates how big str1 should be, or a negative number for some other unexpected error).

 

[xwb]

Assuming gchar is some kind of char

        result = (gchar *)malloc(strlen(str1) + [COLOR=red]strlen(str2)[/COLOR] + 1);

sizeof(gchar) returns the size of one gchar. May not necessarily be the size of the contents of str2.

sizeof(str2) will return the size of a pointer: this is a very common mistake when switching between pointers and arrays.

 

[guillermoandres]

Thank you very much.

gchar corresponds to the standard C char type and is defined in the Glib library.

What I want to do with this two functions is that if I have

gchar *str1 ="Example";
gchar *str2 ="Append";

while *str2 is different of CR (carriage return) and LF (line feed) append to str1 just one char of str2. So in the first loop

str1 = "ExampleA"

second loop

str2 = "ExampleAp"

third loop

str2 = "ExampleApp"

and so on.


I really dont understand how to leave the memory allocation to the caller ?? Using realloc() ?? where can I find a example??

I'm not completely sure but if I do this

result = malloc(strlen(str1) + strlen(str2) + 1);

wouldnt I be allocating extra memory since all I want to do is concatenate to the str1 just one char of str2 at a time??

About losing the data pointer, right now the code is doing what I want. But I'm concern about memory leaking.

Thank you all, and excuse for my bad English.

 

[guillermoandres]

Sorry I meant was:

str1 = "ExampleA"

second loop

str1 = "ExampleAp"

third loop

str1 = "ExampleApp"

and so on.

 

[cpjust]

all I want to do is concatenate to the str1 just one char of str2 at a time

Why on earth would you want to do that? strcat() will append the whole string for you.

 

[guillermoandres]

Because I need to check every character before concatenate it.

Is there another (better) way to do this??

 

[xwb]

You could preallocate the space before the loop since you know roughly how much you will be using and then add them one at a time without reallocating. Unless you're working on one of those microcontrollers with very limited RAM (eg 512K), savings of a few bytes aren't worth worrying about. The program will spend all its time reallocating and hardly any time doing what it is meant to do.

 

[guillermoandres]

Ok I understand your proposal but the problem is that I need to print out the string in a graphical interface, so it's important for me to print only the important information and not a lot of blank spaces.

 

[Largins421]

An elegant way to handle allocation and deallocation of memory is to containerize it in an object. The object can have all generally used buffers, etc. allocated at initialization, and then destroyed when the program is finished. This is more than you asked for, but you might like it. Don't do this for fields used only once in a subroutine, those you should allocated when needed, and deallocated once out of scope, but generally the method is:

1.) In your main include file mainincl.h

#ifndef main__h__
#define main__h__
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdarg.h>
// etc. etc. etc

#ifndef bool
#define bool int
#endif

#ifndef TRUE
#define TRUE 1
#define FALSE 0
#endif

#define BuffSz1 512
#define BuffSz2 1024

typedef struct
{
char *InBuff;
char *OutBuff;
FILE *In;
char *InFileName;
FILE *Out;
char *OutFileName;
int argc;
char **argv;
} sysinfo;

// Function prototypes
int main(int argc, char **argv);
bool SysInit(sysinfo *SysInfo);
void SysAbort(sysinfo *SysInfo, char *String, int
NumArgs, ...);
void SysFree(sysinfo *SysInfo);

2. The Main.c file:

#include "mainincl.h"

int main(int argc, char **argv)
{
sysinfo *SysInfo = NULL;

SysInfo = (sysinfo *)calloc(1, sizeof(sysinfo);
SysInfo->argc = argc;
SysInfo->argv = argv;

if(SysInit(SysInfo))
SysAbort("Unable to initialize", 0);
DoStuff(SysInfo);
SysFree(SysInfo);

free(SysInfo);
}

3. The initialization routine (SysInit.c):
#include "mainincl.h"

bool SysInit(sysinfo *SysInfo)
{
char *ArgPtr = NULL;
char Argid = 0;
int i = 0;
bool RetVal = FALSE;

for(i = 1; i < SysInfo->argc; i++)
{
if(SysInfo->argv[0] == '-')
{
Argid = SysInfo->argv[1];
if(!SysInfo->argv[2])
{
i++;
ArgPtr = SysInfo->argv;
}
else
ArgPtr = (char *)&SysInfo->argv[2];
switch (Argid)
{
case 'i':
SysInfo->InFileName = ArgPtr;
break;
case 'o':
SysInfo->OutFileName = ArgPtr;
break;
default:
SysAbort(SysInfo, "Invalid parameters", 0);
break;
}
}
}

SysInfo->InBuff = (char *)calloc(1, SysInfo->BuffSz);
if(SysInfo->InBuff == NULL)
SysAbort(SysInfo,
"Unable to allocate space for InBuff", 0);

SysInfo->OutBuff = (char *)calloc(1, SysInfo->BuffSz);
if(SysInfo->OutBuff == NULL)
SysAbort(SysInfo,
"Unable to allocate space for OutBuff", 0);

SysInfo->In = fopen(SysInfo->InFileName, "r");
if(SysInfo->In == NULL)
SysAbort(SysInfo, "Unable to open input file %s", 1,
SysInfo->InFileName);
// any other initialization stuff here.
}

3.) The SysAbort routine:
#include "mainincl.h"

void SysAbort(sysinfo *SysInfo, char *String, int
NumArgs, ...)
{
va_list ap;
va_start(ap, NumArgs);

vfprintf(stderr, String, ap);

va_end(ap);

// Check if n\memory needs to be cleaned up
SysFree(SysInfo);

free(SysInfo);
exit(1);
}

4.) SysFree
#include "mainincl.h"

void SysFree(sysinfo *SysInfo)
{
if(SysInfo->Out != NULL)
fclose(SysInfo->Out);
if(SysInfo->In != NULL)
fclose(SysInfo->In);
if(SysInfo->OutBuff != NULL)
free(SysInfo->OutBuff);
if(SysInfo->InBuff != NULL)
free(SysInfo->InBuff);
}

5.) Then your DoStuff

#include "mainincl.h"
void DoStuff(sysinfo *SysInfo)
{
// whatever you would like to do.
}

With this basic framework, you can write any program that you want. What you pass to each module is kept to the minimum, each module can access any information such as files, etc, this makes your code very easy to understand, and best of all, it is easy to organize all of the necessary info, which is available into a single container.

Hope I didn't overstep my bounds, and also hope you enjoy.

Larry (Programming since 1968, and still going)

 

[guillermoandres]

Its way more than I asked for. I got lost in the code. My program its quite long and I dont understand how can I integrate it with what you have written.

Please, if you can, try to explain it a little bit more.

Thanks a lot anyway.

 

[Largins421]

Basicly this is a shell or wrapper if you will that can be used for just about any piece oc C code that you would write. It keeps the allocation and deallocation of strings, arrays, or whatever dynamic code you need to have availabe between (not contained within) various functions. This is a lot like the way a C++ object is created. All of the 'global (if I can use that dirty word)' allocation is done in SysInit, and deallocation in SysFree. By passing the structure 'SysInfo' to subroutines (functions), you automatically pass access to everything within the sysinfo structure. This is not limited, you can build arrays, linked lists or whatever in the SysInfo structure, and all functions immediately have access to this information.

For example, let's say all you want to do is open a text file and print the contents. All you
need is a simple DoStuff.c file as follows, (also comment out the out file in SysInit as you don't
need an output file here).

#include "mainincl.h"

void DoStuff(sysinfo *SysInfo)
{
while(TRUE)
{
fgets(SysInfo->InBuff, SysInfo->BuffSz, SysInfo->In);
if(feof(SysInfo->In))
break;

fprintf(stdout, SysInfo->InBuff);
}
}

That's it! to execute, you would type:
WhaterTheNameIs.exe -i InputTextFileName
, and away you go. No worry about allocation of the input buffer, it's already done in SysInit.

Here's the working and tested code (6 files total)

----------------------------------------------------------
mainincl.h
----------------------------------------------------------
#ifndef main__h__
#define main__h__
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdarg.h>
// etc. etc. etc

#ifndef bool
#define bool int
#endif

#ifndef TRUE
#define TRUE 1
#define FALSE 0
#endif

#define BuffSz1 512
#define BuffSz2 1024

typedef struct
{
char *InBuff;
int BuffSz;
char *OutBuff;
FILE *In;
char *InFileName;
FILE *Out;
char *OutFileName;
int argc;
char **argv;
} sysinfo;

// Function prototypes
int main(int argc, char **argv);
bool SysInit(sysinfo *SysInfo);
void SysAbort(sysinfo *SysInfo, char *String, int
NumArgs, ...);
void SysFree(sysinfo *SysInfo);
void DoStuff(sysinfo *SysInfo);
#endif
----------------------------------------------------------
Main.c
----------------------------------------------------------
#include "mainincl.h"

int main(int argc, char **argv)
{
sysinfo *SysInfo = NULL;

SysInfo = (sysinfo *)calloc(1, sizeof(sysinfo));
SysInfo->argc = argc;
SysInfo->argv = argv;

if(SysInit(SysInfo))
SysAbort(SysInfo, "Unable to initialize", 0);
DoStuff(SysInfo);
SysFree(SysInfo);

free(SysInfo);
}
----------------------------------------------------------
SysInit.c
----------------------------------------------------------
#include "mainincl.h"

bool SysInit(sysinfo *SysInfo)
{
char *ArgPtr = NULL;
char Argid = 0;
int i = 0;
bool RetVal = FALSE;

for(i = 1; i < SysInfo->argc; i++)
{
if(SysInfo->argv[0] == '-')
{
Argid = SysInfo->argv[1];
if(!SysInfo->argv[2])
{
i++;
ArgPtr = SysInfo->argv;
}
else
ArgPtr = (char *)&SysInfo->argv[2];
switch (Argid)
{
case 'i':
SysInfo->InFileName = ArgPtr;
break;
case 'o':
SysInfo->OutFileName = ArgPtr;
break;
default:
SysAbort(SysInfo, "Invalid parameters", 0);
break;
}
}
}

SysInfo->InBuff = (char *)calloc(1, SysInfo->BuffSz);
if(SysInfo->InBuff == NULL)
SysAbort(SysInfo,
"Unable to allocate space for InBuff", 0);

/* Don't need an output file for this instance
SysInfo->OutBuff = (char *)calloc(1, SysInfo->BuffSz);
if(SysInfo->OutBuff == NULL)
SysAbort(SysInfo,
"Unable to allocate space for OutBuff", 0);
*/

SysInfo->In = fopen(SysInfo->InFileName, "r");
if(SysInfo->In == NULL)
SysAbort(SysInfo, "Unable to open input file %s", 1,
SysInfo->InFileName);

SysInfo->BuffSz = BuffSz1;

// any other initialization stuff here.
}
----------------------------------------------------------
SysAbort.c
----------------------------------------------------------
#include "mainincl.h"

void SysAbort(sysinfo *SysInfo, char *String, int
NumArgs, ...)
{
va_list ap;
va_start(ap, NumArgs);

vfprintf(stderr, String, ap);

va_end(ap);

// Check if n\memory needs to be cleaned up
SysFree(SysInfo);

free(SysInfo);
exit(1);
}
----------------------------------------------------------
SysFree.c
----------------------------------------------------------
#include "mainincl.h"

void SysFree(sysinfo *SysInfo)
{
if(SysInfo->Out != NULL)
fclose(SysInfo->Out);
if(SysInfo->In != NULL)
fclose(SysInfo->In);
if(SysInfo->OutBuff != NULL)
free(SysInfo->OutBuff);
if(SysInfo->InBuff != NULL)
free(SysInfo->InBuff);
}
----------------------------------------------------------
DoStuff.c
----------------------------------------------------------
#include "mainincl.h"

void DoStuff(sysinfo *SysInfo)
{
while(TRUE)
{
fgets(SysInfo->InBuff, SysInfo->BuffSz, SysInfo->In);
if(feof(SysInfo->In))
break;

fprintf(stdout, SysInfo->InBuff);
}
}
----------------------------------------------------------

As you can see, in order to create a file list program, all that was needed was the code in DoStuff.c. If you wanted to create a copy program, where you added line numbers to the beginning of the file, all you need is:

1. Since you do need an output file, uncomment the stuff for SysInfo->Out in SysInit.c

2. Create a DoStuff.c as follows:
----------------------------------------------------------
DoStuff.c
----------------------------------------------------------
#include "mainincl.h"

void DoStuff(sysinfo *SysInfo)
{
long linect = 1;

while(TRUE)
{
memset(SysInfo->InBuff, 0, SysInfo->BuffSz);
memset(SysInfo->OutBuff, 0, SysInfo->BuffSz);
fgets(SysInfo->InBuff, SysInfo->BuffSz, SysInfo->In);
if(feof(SysInfo->In))
break;

sprintf(SysInfo->OutBuff, "%d %s", linect++, SysInfo->InBuff);
fputs(SysInfo->OutBuff, SysInfo->Out;
}
}
----------------------------------------------------------
And it's done
To execute this one:
WhaterTheNameIs.exe -i InputTextFileName -o OutputFileName

Hope this makes it a little clearer.