Best method to alloc memory for string and desalloc.

how the Best method to alloc memory for string and after desalloc.

Im using this:

Code:

---

void ClearStr ( char * pString )
{
  *pString = '\0';
}

char *substr(const char *pstr, int start, int numchars)
{
        char *pnew = malloc(numchars+1);
       
        strncpy(pnew, pstr + start, numchars);
        pnew[numchars] = '\0';
        return pnew;
}

char *teste;
char *teste2;

teste = ReceiveComm();
teste2 = substr (teste, 1, 6);

ClearStr (teste);

---

this is executed all 5 minutes, for all long program be executed.

Quote:

---

Originally Posted by sergioms

this is executed all 5 minutes, for all long program be executed.

---

I suspect that most of the time was spent in ReceiveComm(). Are you sure that you have correctly identified the bottle necks in your code? Note that ClearStr() only sets the string to an empty string but does not actually deallocate the memory, but presumably that was intentional.

yes, this was intentional.

Code:

---

char* funcTest(){
      char *rxteste;

      rxteste = substr (teste, 1, 6);
      return rxteste ;
}

---

A Question, if a call a function funcTest, the variable rxteste is alloced and desalloced when the function end ?

It depends on what you mean by "desalloced" -- you still own the memory, since you malloc'ed it and never freed it. The only thing gone is the name rxteste.

Quote:

---

Originally Posted by sergioms

yes, this was intentional.

Code:

---

char* funcTest(){
      char *rxteste;

      rxteste = substr (teste, 1, 6);
      return rxteste ;
}

---

A Question, if a call a function funcTest, the variable rxteste is alloced and desalloced when the function end ?

---

rxteste is allocated in substr, but not de-allocated - and you can't deallocate it here either, since you are returning it to the calling function - and if you deallocate it there, it would not be valid later on (in the calling function).

--
Mats

and, when I would to use malloc ?

in "Global Variables?"

and when I need a string with value all time changed.?

Quote:

---

Originally Posted by matsp

rxteste is allocated in substr, but not de-allocated - and you can't deallocate it here either, since you are returning it to the calling function - and if you deallocate it there, it would not be valid later on (in the calling function).

--
Mats

---

ok, in this case
when i alloc in substr, where and how to de-alloc this ?

sorry for my english, and my inconvenience.

Quote:

---

Originally Posted by sergioms

when i alloc in substr, where and how to de-alloc this ?

---

You would deallocate the memory when you no longer need to use it by using free(), which is also from <stdlib.h>.

Quote:

---

Originally Posted by sergioms

ok, in this case
when i alloc in substr, where and how to de-alloc this ?

sorry for my english, and my inconvenience.

---

From the back: Don't worry about your English - it's better than my italian or whatever you speak natively...

You need to dealloc whereever the string is no longer needed. You haven't shown all the code [you don't need to show us all code - just that I can't tell you what the right place is when I can't see it - but I can describe it], so I don't know what it is that you do with the string.

--
Mats

ok,

I have a string receiving text at 200 milleseconds. (strReceiveCom)

This String is processed as a packet

Example: [2]FUNC[9]VALUE1[9]VALUE 2[9][13][10]

*where: [] indicates a decimal value in ASCII.

This packet, is splited in others strings:

strPckFunction = substr (strReceiveCom, 1,4);
strValue1 = substr (strReceiveCom, 6, iTabPos);

the strings strPckFunction and strValue1 is processed locally and after the strReceiveCom value need cleaned to receive more text.

So, do you have enough time to process the value in 200ms every time, or do you need to build up a queue of elements and then process then later?

--
Mats

No, I dont need to build up a queue, I Can clean actual strReceiveCom and process the last packet, I can lose all old text.

So basically, when you are finished with strValue1 and strPckFunction, you should free both of those strings.

Although I would probably prefer to not allocate memory at all, instead use a local char array to store the value and function strings in, and pass that to the substr() function as an argument. That reduces the overhead of calling malloc and free, as well as reducing the chance of "forgetting" to free the string.

--
Mats

Code:

---

char *substr(char **Pnew, const char *pstr, int start, int numchars)
{
        strncpy(pnew, pstr + start, numchars);
        pnew[numchars] = '\0';
        return pnew;
}

void Test (){
        char *pnew = malloc(numchars+1);

                substr(&pnew, "TESTE", 0,2) ;

}

---

is this ?

I'm wondering if part of your problem has to do with "returns address of local variable"...anyway, if you know that your string is not going to exceed a certain size, rather than returning a malloc'd pointer, you can pass the string in as an argument instead. Even with with malloc, this may be easier since then string will be allocated and free'd in the same place:

Code:

---

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

void substring (char *string, char *substr, int numchars) {
        strncpy(substr,string,numchars);
        substr[numchars+1]='\0';
}       

int main() {
        char string[]="ciao mondo sergio", sub[6],
                *sub2=malloc(7);
        substring(&string[11],sub,6);                // without malloc
        printf("%s\n",sub);
        substring(&string[5],sub2,5);                // with malloc...
        printf("%s\n",sub2);
        free(sub2);                                // ...free when finished
        return 0;
}

---

Output:

Code:

---

sergio
mondo

---

Notice the string is passed in starting with the address of the first character of the substring, rather than using int start (a possibility).

Of course in my example, function substring() ends up being an anagram of strncpy, but hopefully there's something here you can use.

thank's a lot MK27,

it was very good for me!

Quote:

---

Code:

---

char *
    strncpy(char *restrict s1, const char *restrict s2, size_t n);

---

The strncpy() function copies at most n characters from s2 into s1. If
s2 is less than n characters long, the remainder of s1 is filled with
`\0' characters. Otherwise, s1 is not terminated.

---

From:
http://www.manpagez.com/man/3/strncpy/
(My bold)

--
Mats

Quote:

---

If s2 is less than n characters long, the remainder of s1 is filled with
`\0' characters. Otherwise, s1 is not terminated.

---

Oops! My mistake. Sorry. Thanks matsp. I'll edit my last post...

Quote:

---

Originally Posted by MK27

Oops! My mistake. Sorry. Thanks matsp. I'll edit my last post...

---

in this case,

Code:

---

int sLen;

sLen = strlen(strText);
strText [sLen ] = '\0';

---

yes?

Quote:

---

Originally Posted by sergioms

in this case,
[...]
yes?

---

No. strlen looks for the \0, so there is never a point in doing that. If strncpy did not add a null terminator, then strlen will give an incorrect result.

Of course, you could always just provide an extra character so that strncpy will add a \0.

Code:

---

substring(&string[11],sub,7);               
substring(&string[5],sub2,6);

---

Code:

---

void substring (char *string, char *substr, int numchars) {
        strncpy(substr,string,numchars);
        substr[numchars+1]='\0';
}

---

This is MK27's corrected code - I'm not sure if you should have the +1 there or not - I think not. But either way, something like that is correct.

--
Mats

Quote:

---

Originally Posted by matsp

Code:

---

void substring (char *string, char *substr, int numchars) {
        strncpy(substr,string,numchars);
        substr[numchars+1]='\0';
}

---

This is MK27's corrected code - I'm not sure if you should have the +1 there or not - I think not. But either way, something like that is correct.

--
Mats

---

you should have the '+1' because otherwise it would be replacing the last character of the string with a 'string-termination'.

and you could simply do what MK said, which is to pass the parameters of strncopy with an extra character in the 'character-length' parameter which would end up like this:

Code:

---

void substring (char *string, char *substr, int numchars) {
        strncpy(substr,string,numchars+1);
}

---

Quote:

---

Originally Posted by abraham2119

you should have the '+1' because otherwise it would be replacing the last character of the string with a 'string-termination'.

and you could simply do what MK said, which is to pass the parameters of strncopy with an extra character in the 'character-length' parameter which would end up like this:

Code:

---

void substring (char *string, char *substr, int numchars) {
        strncpy(substr,string,numchars+1);
}

---

---

in this case, the function always copy +1 char,
ex: substring ("TESTE", strtest, 1)

output: TE

Quote:

---

Originally Posted by sergioms

in this case, the function always copy +1 char,

---

Yep. Which means that other version of mine would cause the same problem. So you do have to add a null terminator in that function.

Code:

---

                iPosStart = strpos (pPacote, cSTX);
        iPosStart++;
       
        iPosEnd = strpos (pPacote, cCR);
       
        iPosTam = iPosEnd - iPosStart;
       

        if((iPosStart >= 0) && (iPosEnd)){
                substring(&pPacote[iPosStart], cPacote, iPosTam);

                VcomSendSocket(iUserSocket, cPacote, strlen (cPacote));
       
                if(strlen (cPacote) > 0){
                                               

                        substring(&cPacote[0], cFuncao, 2);

                        VcomSendSocket(iUserSocket, cFuncao, strlen (cFuncao));
                       
                        if (strcmp (cFuncao, "OP") == 0){                                                                // Recebe Ordem de Pesagem
                                                                substring(&cPacote[3], cSubFuncao, iPosTam - 2);
                                                                VcomSendSocket(iUserSocket, cSubFuncao, strlen (cSubFuncao));
                                OrdemProducao (cSubFuncao);

---

pPacote: OP F04 686693 1 BL1 MASSA MONOPOROS

Output (VcomSendSocket):

6/1/2009 09:39:16 - Rec: PASSO 1[NULL]
6/1/2009 09:39:16 - Rec: CC F04 686693 1 BL1 MASSA MONOPOROS�PASSO 2CC�[NULL]

i want to get only the middle string without null and others chars.

:S

like my old function:

Code:

---

char *substr(const char *pstr, int start, int numchars)
{
        char *pnew = malloc(numchars+1);
       
        strncpy(pnew, pstr + start, numchars);
        pnew[numchars] = '\0';
        return pnew;
}

---

Code:

---

void substring (char *string, char *substr, int numchars) {
        strncpy(substr,string,numchars);
        substr[numchars+1]='\0';
}

---

should work, but I still think that if numchars is the actual number of characters you want to copy [which I think it should be], then you should set the zero to indicate the end at [numchars], not [numchars+1], since C arrays start at zero, and [numchars] really is the (numchars+1)th element - e.g. if we have a length of 2, then [2] is the third element of the array.

--
Mats

Quote:

---

Originally Posted by matsp

Code:

---

void substring (char *string, char *substr, int numchars) {
        strncpy(substr,string,numchars);
        substr[numchars+1]='\0';
}

---

should work, but I still think that if numchars is the actual number of characters you want to copy [which I think it should be], then you should set the zero to indicate the end at [numchars], not [numchars+1], since C arrays start at zero, and [numchars] really is the (numchars+1)th element - e.g. if we have a length of 2, then [2] is the third element of the array.

--
Mats

---

Yes, Now is correct, thanks again.

There is someone command to clear all allocated memory ?

Quote:

---

Originally Posted by sergioms

There is someone command to clear all allocated memory ?

---

You mean "free everything", - no there is no such thing - you couldn't meaningfully make use of such a function anyways, because it would require that no functions that you ever call to the C library or such uses malloc - which they may possibly do.

--
Mats

humm

then, I need use malloc always, to all strings ?

Code:

---


void ProcessaBuffer (){

        unsigned char *cPacote, *cPeso;
        int iCont;
        int iPosStart, iPosEnd, iPosTam;

        iPosStart = strrpos (InBuf, cSTX);
        if(iPosStart >= 0)
                iPosStart++;
       
        iPosEnd = strrpos (InBuf, cCR);
       
        iPosTam = iPosEnd - iPosStart;
               
        if((iPosStart >= 0) && (iPosEnd) && (iPosTam == 15)){
               
                substring(&InBuf[iPosStart], cPacote, iPosTam);
                substring(&cPacote[1], cStatus, 1);
                substring(&cPacote[3], cPeso, 6);
                                                                       
                lPeso = strtod (cPeso, NULL);
        }
       
        ClearStr (InBuf);
}

---

in this code, this function is called at 200 ms, always.

unsigned char is better then char ?

unsigned or signed char should only make a difference if you are:
1. comparing (unsigned has no negative values, so all characters are in a uniform range 0..255, whilst characters that are above 127 become negative values in signed form).
2. When converting to (signed) integer - an unsigned will retain the same 0..255 value, whilst the signed char will give a value -128..127.

You certainly do not need to allocate space for all strings - you only need to allocate space if you:
1. Use A LOT of space (several kilobytes or more)
2. Do not know how much space you need, and the largest amount is VERY large compared to the smallest amount [particularly if you have many strings and want to conserve space].

Using

Code:

---

char str[SIZE];

---

will create a string of SIZE elements, which should work for cases where SIZE can be determined before you run the program - and as long as the largest size you need is less than (say) a few hundred bytes, it's the best way to get space for a string.

--
Mats

but the most part is string with a variable size,

and with resize (strcat, strcpy, substring....)

my ICP-i7188E3 seems behave better with allocation, in some this cases.

Quote:

---

Originally Posted by sergioms

but the most part is string with a variable size,

and with resize (strcat, strcpy, substring....)

my ICP-i7188E3 seems behave better with allocation, in some this cases.

---

Of course the string can be different length - as long as you know how large IT CAN BE, you will be fine with a locally allocated string.

What do you mean "behave better"? malloc is definitely a few dozen instructions in the average/good case, and thousands of instructions in the "poor" case. stack memory allocation is at most one extra instruction - most of the time none, as it is just part of the overall local variable allocation that you have whenever there is any local variable, so essentially free. And there is no need to worry about loosing track of the memory allocated, because when you return from the function where the variable is declared, it goes away all by "magic".

--
Mats

ok, I undestand.

in theory I accept this, but my small memory continues full.

whatever, now the program execute by 15 minutes, before your tips, only 3.

at someone point, the code is incorrect, and make this overflow.

But im in a bad place, without a debug, and with poot know about C strings, pointers and memory [:S]

I will try use coreleft to verify my remain memory....

Right, so using local variables will allow you to NOT run out of memory, since you are not allocating more and more memory, you are reusing the same bit of memory. The drawback is that you would need to pass the string around from the very base-function that is the "lowest in the call-stack", otherwise you loose the strings you have picked out of the bigger package.

--
Mats

is any difference of memory RAM to SRAM (in code)

Now i change the program to always alloc variable (include local)
and the program be executing for 1 hour.

*alloc / free

its crazy :S

Quote:

---

Originally Posted by sergioms

is any difference of memory RAM to SRAM (in code)

Now i change the program to always alloc variable (include local)
and the program be executing for 1 hour.

*alloc / free

its crazy :S

---

SRAM is a class of RAM, the other type is DRAM. The difference is that SRAM is "static", which means that as long as the power is applied, it retains it's content. DRAM is dynamic, and it needs refreshing every now and again (per RAM it retains it's contents for a few milliseconds - normally a row is refreshed every 16 microseconds or so). SRAM is also much easier to interface to the processor, as it has a more direct interface (DRAM being made as cheap as possible has a more compressed set of pins, and address to the content has to be addressed in two steps, once for the row, and once for the column within that row).

And I'd guess you "fixed" something else while at it - or you were using memory from a function that had returned, which is a not so unusual programming error by beginners.

--
Mats

Quote:

---

Originally Posted by matsp

SRAM is a class of RAM, the other type is DRAM. The difference is that SRAM is "static", which means that as long as the power is applied, it retains it's content. DRAM is dynamic, and it needs refreshing every now and again (per RAM it retains it's contents for a few milliseconds - normally a row is refreshed every 16 microseconds or so). SRAM is also much easier to interface to the processor, as it has a more direct interface (DRAM being made as cheap as possible has a more compressed set of pins, and address to the content has to be addressed in two steps, once for the row, and once for the column within that row).

And I'd guess you "fixed" something else while at it - or you were using memory from a function that had returned, which is a not so unusual programming error by beginners.

--
Mats

---

Last code Version:

http://rafb.net/p/8f1Uh382.html

the problem has come back again.

somebody alive?

Code:

---

void EscreveDisplay (unsigned char *pMensagem){
        unsigned char cDispPacote [51];
        unsigned int iLims;
        unsigned int iLenMsg, iLenCont, iContIndex;
        int iBcc;

        iLims  = 46;
       
        cDispPacote [0] = 129;
        cDispPacote [1] = 126;
        cDispPacote [2] = iLims % 256;
        cDispPacote [3] = iLims / 256;
        cDispPacote [4] = 1 % 256;
        cDispPacote [5] = 1 / 256;
        cDispPacote [6] = 0 % 256;
        cDispPacote [7] = 0 / 256;
        cDispPacote [8] = 76;
        cDispPacote [9] = 202;
               
        iLenMsg = strlen (pMensagem);

        if (iLenMsg > 40)
                iLenMsg = 40;

        if (iLenMsg < 40){
                for(iLenCont=iLenMsg; iLenCont<40; iLenCont++)
                        strcat (pMensagem, ".");
        }
       
        for(iLenCont=0; iLenCont<40; iLenCont++){
                iContIndex = iLenCont + 10;
                cDispPacote [iContIndex] = pMensagem [iLenCont];
        }
       
        iBcc = cDispPacote [4];
        cDispPacote [50] = iBcc;
        cDispPacote [51] = '\0';
       
        for(iContIndex=0; iContIndex<=51; iContIndex++){
                ToCom (iComDisplay, cDispPacote[iContIndex]);
        }
}

---

pMensagem is a local variable.

but,

Code:

---

for(iLenCont=iLenMsg; iLenCont<40; iLenCont++)
                        strcat (pMensagem, ".");

---

the size is changed many times,

this is a wrong code, or unsafe?

Overall

a few comments on the code:

Code:

---

        // Transforma os Int em Char (ASCII)
        cSTX = STX;
        cCR  = CR;
        cLF  = LF;
        cTab = TAB;

---

What's the point of this - the C language will automatically convert from integer constant to char when needed. If you NEED to indicate a number constant is a char and not a integer, you can use '\nnn' where nnn is an octal number. But the use of these things, I can't actually see anywhere that you can't just use STX, CR, LF or TAB.

You are using an awful lot of global variables - that is really not a good idea.

Code:

---

        //if (cPacote == NULL)
        //        EscreveDisplay ("ALLOC");

---

You probably DO want to check that allocations works.

Code:

---

        sprintf (cStr, "OIRS\t%s\t%s\t%d", MATERIAL_DETAIL_ID, CD_MATERIA_PRIMA, lPesoTotal);

...
                sprintf (PesoDisplay,  "%s %d", DS_ABREVIADA, lPesoDecrescente);

---

gcc complains that lPesoTotal is a long, and you are using %d. Similar problem in the second sprintf.

Code:

---

                if (lPesoDecrescente == NULL)
                        lPesoDecrescente = 0;

---

"NULL" is the same as zero (aside from weird details on the subject), so aside from the warning that this compares a pointer to integer, what are you actually trying to achieve here?


Many unused variables:

Code:

---

icp.c: In function `RecebeSerial':
icp.c:212: warning: unused variable `iMaxLocal'
icp.c: In function `DevolvePesagem':
icp.c:307: warning: int format, long int arg (arg 5)
icp.c: In function `PesaItem':
icp.c:461: warning: int format, long int arg (arg 4)
icp.c:474: warning: implicit declaration of function `Delay'
icp.c: In function `OrdemProducao':
icp.c:492: warning: unused variable `iTab'
icp.c: In function `OrdemProducaoItem':
icp.c:540: warning: unused variable `iTab'
icp.c: In function `EscreveDisplay':
icp.c:743: warning: implicit declaration of function `ToCom'
icp.c: In function `ProcessaBuffer':
icp.c:752: warning: unused variable `iCont'

---

(Line numbers may not match your exact code, as I had to hack around missing header files in the source).

Code:

---

    AddUserTimerFunction(UserCount, 200); // Adiciona  Timer para Chamar Função UserCount a cada 100 ms

---

Is 200 representing 200 ms - if so, the comment is wrong, I think.

Code:

---

                if (strlen (InBuf) >= iMaxBuffer - 1){        // Limpa Buffer se Estiver Cheio

---

Make iMaxBuffer an unsigned (or size_t) so that you are not comparing signed with unsigned.

Code:

---

        unsigned char *cFuncao;
        unsigned char *cSubFuncao;
...
                        substring(&cPacote[0], cFuncao, 2);

---

You have not allocated any memory for cFuncao, so it will probably overwrite some random memory location. Same applies for the cSubFuncao substring calls.

Code:

---

void PesaItem (void){
 
        unsigned char *PesoDisplay = malloc (15);
...

---

There is no reason to use malloc here.

Code:

---

void PesaItem (void){
 
        unsigned char PesoDisplay[15];

---

General coding style comments:
You are sometimes adding a space between the function name and the (, at other times you are not. Likewise, variable names are inconsistently starting with i, c, p, etc, and at other times not. And your indentation consists of a mixture of tabs and spaces. Use either one or the other.

--
Mats

thank u so much again,

I was changed the code,

what % I Would to use in sprintf to representate long ?

When I Changed this:

Code:

---

unsigned char cFuncao = malloc (2);
unsigned char cSubFuncao = malloc (2);

---

But now, the program stop execution when has receive "OKOI" (OK cFuncao, OI cSubFuncao):

Code:

---

if(strlen (cPacote) > 0){
                                               
                        substring(&cPacote[0], cFuncao, 2);
                       
                        if (strcmp (cFuncao, "OP") == 0){                                       
                                substring(&cPacote[3], cSubFuncao, iPosTam - 2);
                                OrdemProducao (cSubFuncao);
                        }
                        else if (strcmp (cFuncao, "OI") == 0){                                       
                                substring(&cPacote[2], cSubFuncao, iPosTam - 2);
                                OrdemProducaoItem (cSubFuncao);
                        }
                        else if (strcmp (cFuncao, "OK") == 0){       
                                substring(&cPacote[2], cSubFuncao, 2);
                                if (strcmp (cSubFuncao, "OP") == 0)        {
                                        EnviaPedidoPC ("OISL", 4);               
                                }
                                else if (strcmp (cSubFuncao, "OI") == 0){       
                                        lPesoTara = lPeso;
                                        iPesaItem = 1;

---

Quote:

---

Originally Posted by sergioms

thank u so much again,

I was changed the code,

what % I Would to use in sprintf to representate long ?

---

%ld

Quote:

---

When I Changed this:

Code:

---

unsigned char cFuncao = malloc (2);
unsigned char cSubFuncao = malloc (2);

---

---

Don't you get any warnings?
malloc returns pointer, it cannot be stored in one char var

Quote:

---

But now, the program stop execution when has receive "OKOI" (OK cFuncao, OI cSubFuncao):

---

Don't you need to store the null terminator as well?
For example strcmp function will work only on the nul-terminated strings

And I personally would go with

Code:

---

unsigned char cFuncao[3];
unsigned char cSubFuncao[3];

---

--
Mats

I has changed, with your tips.

http://rafb.net/p/QMeEO360.html

after some (long) time send correct, function EnviaPedidoPC send "................."
But I Don't know the motive.

Maybe, some point of memory has conflicted with function EscreveDisplay, in a part fill with . the string to complete 40 chars.

the program start a process then call. IniciaMemoria to alloc this variables.

Code:

---

void IniciaMemoria (void){
        CD_FABRICA = malloc (5);
        BATCH_ID = malloc (11);
        CD_PESAGEM = malloc (9);
        CD_BALANCA = malloc (5);
        ITEM_DESC1 = malloc (12);
        MATERIAL_DETAIL_ID = malloc (11);
        NR_SEQ_PESAGEM = malloc (3);
        CD_MATERIA_PRIMA = malloc (11);
        DS_ABREVIADA = malloc (11);
        QT_REQUERIDA = malloc (6);
        PR_TOLERANCIA_MIN = malloc (6);
        PR_TOLERANCIA_MAX = malloc (6);
        ID_PESAR = malloc (2);
}

---

then i fill this variables in 2 functions, one is these:

Code:

---

void OrdemProducao (unsigned char *pOP){        //OP        CD_FABRICA        BATCH_ID        CD_PESAGEM        CD_BALANCA        ITEM_DESC1
       
        unsigned char *cValor = NULL;
        int iCont=1;
       
        cValor = strtok(pOP, "\t");        // Procura Pelo TAB
       
        while (cValor){
                        switch (iCont){
                                case 1:
                                        strcpy (CD_FABRICA, cValor);
                                        break;
                                case 2:
                                        strcpy (BATCH_ID,cValor);
                                        break;
                                case 3:
                                        strcpy (CD_PESAGEM,cValor);
                                        break;
                                case 4:
                                        strcpy (CD_BALANCA,cValor);
                                        break;
                                case 5:
                                        strcpy (ITEM_DESC1,cValor);
                                        EscreveDisplay (cValor, 0);
                                        //EscreveDisplay ("                                        ");
                                        break;
                                default:
                                        break;
                        }
                        cValor = strtok(NULL, "\t");
                        iCont++;
        }
       
        iStepInput0 = 2;
        EnviaPedidoPC ("OKOP", 4);
}

---

when I Have a flow complete (process complete) then I Call this function (OrdemProducao) Again.

I need de-allocate this variables, and re-allocate to fill it again (with other values)?

for example:

Code:

---

void LimpaMemoria (void){
        free(CD_FABRICA);
        free(BATCH_ID);
        free(CD_PESAGEM);
        free(CD_BALANCA);
        free(ITEM_DESC1);
        free(MATERIAL_DETAIL_ID);
        free(NR_SEQ_PESAGEM);
        free(CD_MATERIA_PRIMA);
        free(DS_ABREVIADA);
        free(QT_REQUERIDA);
        free(PR_TOLERANCIA_MIN);
        free(PR_TOLERANCIA_MAX);
        free(ID_PESAR);
}

---

You do not need to free and allocate again if you just want to put different values in the variables. In fact, when the value inside malloc() is a constant, there's no reason why you can't just use a char array, as described above. These variables are globals anyways, so there should be absolutely no difference. If there is, it's probably because you are going beyond the end of the array - and using malloc/free, you may have some small margin, which you don't get with the fixed size array [or at least, there will not be any BIG gaps].

--
Mats

Hey Sergio!

If you are doing this many times (malloc then free) it could be part of the reason your program is slow. Since you seem to know how big the strings are going to be, you don't need to do it at all -- Could part of your reason for these and all the globals be that you are having difficulty passing local variables around around?

Consider this: Let's say we want to read a catalogue of goods from a file into some variables for some good reason. This way everyone can understand the goal. Let's say the input file looks like this:

Code:

---

top "The top part" red Bob 13       
side "A side part" green Cleo 2000
corner "Connects 3rd dimension" round MK 27

---

Here's a quick and efficient method that took me a while to type up so I hope you appreciate it:

Code:

---

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <fcntl.h>
#include <unistd.h>

struct template {
        char item[20];
        char description[50];
        char detail[10];
        char manufacturer[12];
        int cost;
};        // wow, structs are great


void fillstruct (struct template *ptr, char *line) {  // pass in a pointer
        char *tok=strtok(line," ");
        strcpy(ptr->item,tok);     
        tok=strtok(NULL,"\"");
        strcpy(ptr->description,tok);     
        tok=strtok(NULL," ");
        strcpy(ptr->detail,tok);     
        tok=strtok(NULL," ");
        strcpy(ptr->manufacturer,tok);     
        tok=strtok(NULL," ");
        ptr->cost=atoi(tok);
}

void showstruct (struct template *ptr) {
        printf("\nDescription of \"%s\": %s\n"
                "Made by: %s\n"
                "Price: $%d\n"
                "note -- %s\n",
                ptr->item, ptr->description, ptr->manufacturer, ptr->cost, ptr->detail);
}


int linein (int fd, char *string) {
        int i=0;
        char ch;
        while (ch!='\n') {
                if (read(fd,&ch,1)!=1) break;
                string[i]=ch;
                i++;
        }
        return i;
}


int main (int argc, char *argv[]) {
        int len,i=0,x, fd=open("example.txt",O_RDONLY);
        char string[128];        // should fit
        struct template *example=malloc(sizeof(*example)); 
       
        while ((len=linein(fd,string))>0) {
                string[len]='\0';
                if (i>0) example=realloc(example,(i+1)*sizeof(*example));
                fillstruct(&example[i],string);
                i++;
        }
        close(fd);
        /* now let's see if this worked */
        for (x=0;x<i;x++) showstruct(&example[x]);
        free(example);
        return 0; 
}

---

If you already know how many products are actually in the catalougue, you can just declare an array of structs like this:

Code:

---

struct template example[3];

---

and, as matsp advises, avoid the use of malloc completely.

I will probably be here for a few hours if you see anything you like :cool:
Output:

Code:

---


Description of "top": The top part
Made by: Bob
Price: $13
note -- red

Description of "side": A side part
Made by: Cleo
Price: $2000
note -- green

Description of "corner": Connects 3rd dimension
Made by: MK
Price: $27
note -- round

---

thanks mk27

but these global variables, filled at 30 minutes, with one record, then processed and get other value again after 30 minutotes.

off topic:

how to reset (restart) an application with command?

Quote:

---

Originally Posted by sergioms

thanks mk27

but these global variables, filled at 30 minutes, with one record, then processed and get other value again after 30 minutotes.

---

And? I expect you have a lot more than 400 bytes of free memory, right? And you are allocating (if my mental addition is correct) 98 bytes - so even if each block takes 4 times more than you actually allocate [and it certainly will take SOME more memory - how much is hard to say], then the space is enough.

Quote:

---

off topic:

how to reset (restart) an application with command?

---

That is not as easy as it sounds. You need to exit your current app too, and that is the part that makes it messy. In the past I have had a batch-file that starts the application, and then rewritten the batchfile from inside the application to make it "continue with other stuff", which DOES work, but it's relying on a batch file being there and being writable, so it's not a particuluarly neat idea, really.

--
Mats

Yes, I Complete a process with sucessufull, then process stop a execution for ~30 minutes, then the operator press the Digital I/O button, and the process start again.

But, The System send a call to PC, and Receive a Response, this response is processed (OrdemProducao) and the process start again, but at this moment I Send to PC a confirmation, and the value is "............", but it's not correct.

I think solve this case with restart equipament (the program is automatic executed), or restart application, or free all memory.

But i dont know how to make it.

ok, please lets will continue here then....

what u suggest to work with this Global Variables, filled in OrdemProducao function.
When when I need Clean up this.

sorry, some text i dont undestand (because my english)

Right. Freeing the memory if there is nothing else going on for 30 minutes is a waste of programming - because the memory is not needed for something else, right?

As to your problem with sending incorrect response: You have a bug in the code - fix that - not by restarting the application, because as you work along (making other changes to your code), you will most likely find something else that fails and can't be solved by restarting the application, because it needs some information that is not available after you restart the application.

As a dummy attempt to see if it helps, increase the size of your character arrays (memory allocations) by double - if that works, then you know that you are going over the limit on some array. If it works, remove half of the doublings, and try again. If that recreates the problem, you know that it is one of those variables. Keep doing this until you have narrowed down which variable(s) it is that causes the actual problem. Then study all the code that accesses the variable.

I'm going to go over your new code in a bit, and I will post comments later on. [I will pay special attention to memory allocations and potential overwriting the end].

--
Mats

Right a few things:
I think I have found (at least one of) your bug(s):

Code:

---

void EscreveDisplay (unsigned char *pMensagem, unsigned int pDelaySec){
        unsigned char cDispPacote [51];
        int iComDisplay = 2;
....
 
        iBcc = cDispPacote [4];
        cDispPacote [50] = iBcc;
        cDispPacote [51] = '\0';
 
...
}

---

You are going to the 52nd element of a 51 element array - not good.

Code:

---

void IniciaMemoria (void){
        CD_FABRICA = malloc (5);
        BATCH_ID = malloc (11);
        CD_PESAGEM = malloc (9);
        CD_BALANCA = malloc (5);
        ITEM_DESC1 = malloc (12);
        MATERIAL_DETAIL_ID = malloc (11);
        NR_SEQ_PESAGEM = malloc (3);
        CD_MATERIA_PRIMA = malloc (11);
        DS_ABREVIADA = malloc (11);
        QT_REQUERIDA = malloc (6);
        PR_TOLERANCIA_MIN = malloc (6);
        PR_TOLERANCIA_MAX = malloc (6);
        ID_PESAR = malloc (2);
}

---

You should probably check that malloc doesn't return NULL. You even have a function that does that:

Code:

---

void AllocMemoria (unsigned char **pVariavel, unsigned int pTamanho)

---

Code:

---

        cSTX = 2;
        cCR  = 13;
        cLF  = 10;
        cTab = 9;

---

You did the exact opposite of what I meant - you should remove the cCR, cSTX and use the STX, CR defines that you had in the original code.

Code:

---

void ProcessaBuffer()
...
        unsigned char *cStatus;                                                                // Indica se o Peso atual está em movimento ou é estabilizado
        ...
        unsigned char *cPeso;

---

cStatus and cPeso are never allocated any memory - that is definitely bad. I'd make them local arrays, rather than dynamically allocate them, as they are not used outside of the function itself. That applies to cPacote as well. cStatus should be 2 in size, cPeso should be 7.

Similarly,

Code:

---

int ProcessaChamado():
...
        unsigned char *cFuncao;
        unsigned char *cSubFuncao;

---

are also not allocated. This is very likely to cause all sorts of problems.

Code:

---

void PesaItem(void):
...
                sprintf (PesoDisplay,  "%s %ld", DS_ABREVIADA, lPesoDecrescente);

---

PesoDisplay is 17 bytes long. DS_ABREVIADA is 11. lPesoDecrescente is a long number, and there si a space between the %s and %ld - what if lPesoDescente is more than 999999?

Code:

---

                                        strcpy (DS_ABREVIADA,cValor);

---

You are not actually checking that cValor fits in DS_ABREVIADA - if it's longer than 10, then it won't fit. (there are several other cases - I just looked back for this string, since I was trying to validate that PesoDisplay above was OK.

--
Mats

OK, lets goo
I work at last 20 hours withou sleep hehehe

verify if is what i was undestand....

Quote:

---

You are going to the 52nd element of a 51 element array - not good.

---

Then I Have to declare with unsigned char cDispPacote [52]; ?

Code:

---

void IniciaMemoria (void){
        CD_FABRICA = malloc (5);
        BATCH_ID = malloc (11);
        CD_PESAGEM = malloc (9);
        CD_BALANCA = malloc (5);
        ITEM_DESC1 = malloc (12);
        MATERIAL_DETAIL_ID = malloc (11);
        NR_SEQ_PESAGEM = malloc (3);
        CD_MATERIA_PRIMA = malloc (11);
        DS_ABREVIADA = malloc (11);
        QT_REQUERIDA = malloc (6);
        PR_TOLERANCIA_MIN = malloc (6);
        PR_TOLERANCIA_MAX = malloc (6);
        ID_PESAR = malloc (2);

        if (CD_FABRICA == NULL)
                EscreveDisplay ("ALLOC ERROR 1");
               
        if (BATCH_ID == NULL)
                EscreveDisplay ("ALLOC ERROR 2");
               
        if (CD_PESAGEM == NULL)
                EscreveDisplay ("ALLOC ERROR 3");

        if (CD_BALANCA == NULL)
                EscreveDisplay ("ALLOC ERROR 4");
               
        if (ITEM_DESC1 == NULL)
                EscreveDisplay ("ALLOC ERROR 5");
               
        if (MATERIAL_DETAIL_ID == NULL)
                EscreveDisplay ("ALLOC ERROR 6");
               
        if (NR_SEQ_PESAGEM == NULL)
                EscreveDisplay ("ALLOC ERROR 7");
               
        if (CD_MATERIA_PRIMA == NULL)
                EscreveDisplay ("ALLOC ERROR 8");
               
        if (QT_REQUERIDA == NULL)
                EscreveDisplay ("ALLOC ERROR 9");
               
        if (PR_TOLERANCIA_MIN == NULL)
                EscreveDisplay ("ALLOC ERROR 10");
               
        if (PR_TOLERANCIA_MAX == NULL)
                EscreveDisplay ("ALLOC ERROR 11");
               
        if (ID_PESAR == NULL)
                EscreveDisplay ("ALLOC ERROR 12");       

}

---

The

Code:

---

/*
        cSTX = 2;
        cCR  = 13;
        cLF  = 10;
        cTab = 9;
*/

#define cSTX 2
#define cCR 13
#define cLF  10
#define cTab 9

---

Code:

---

void ProcessaBuffer()
...
        unsigned char *cStatus;       
        unsigned char *cPeso;

---

when I Tried to alloc in this function always the application cause error, but now this function is run fine.

Code:

---

int ProcessaChamado():
...
        unsigned char *cFuncao;
        unsigned char *cSubFuncao;

---

same case of above

Code:

---

void PesaItem(void):
...
                sprintf (PesoDisplay,  "%s %ld", DS_ABREVIADA, lPesoDecrescente);

---

the driver truck have at most 25000 (Kg)

Code:

---

                                        strcpy (DS_ABREVIADA,cValor);

---

Quote:

---

You are not actually checking that cValor fits in DS_ABREVIADA - if it's longer than 10, then it won't fit. (there are several other cases - I just looked back for this string, since I was trying to validate that PesoDisplay above was OK.

---

sorry,
this i dont undestand

Quote:

---

Originally Posted by sergioms

OK, lets goo
I work at last 20 hours withou sleep hehehe

---

From experience I can say that this is not good in the longer term. Often you end up spending time fixing problems caused by your inability to think whilst really tired, and end up making more mistakes than you would if you slept more - so sleeping is actually better for your code.

Quote:

---

verify if is what i was undestand....



Then I Have to declare with unsigned char cDispPacote [52]; ?

---

Yes.

Quote:

---

Code:

---

void IniciaMemoria (void){
        CD_FABRICA = malloc (5);
        BATCH_ID = malloc (11);
        CD_PESAGEM = malloc (9);
        CD_BALANCA = malloc (5);
        ITEM_DESC1 = malloc (12);
        MATERIAL_DETAIL_ID = malloc (11);
        NR_SEQ_PESAGEM = malloc (3);
        CD_MATERIA_PRIMA = malloc (11);
        DS_ABREVIADA = malloc (11);
        QT_REQUERIDA = malloc (6);
        PR_TOLERANCIA_MIN = malloc (6);
        PR_TOLERANCIA_MAX = malloc (6);
        ID_PESAR = malloc (2);

        if (CD_FABRICA == NULL)
                EscreveDisplay ("ALLOC ERROR 1");
               
        if (BATCH_ID == NULL)
                EscreveDisplay ("ALLOC ERROR 2");
               
        if (CD_PESAGEM == NULL)
                EscreveDisplay ("ALLOC ERROR 3");

        if (CD_BALANCA == NULL)
                EscreveDisplay ("ALLOC ERROR 4");
               
        if (ITEM_DESC1 == NULL)
                EscreveDisplay ("ALLOC ERROR 5");
               
        if (MATERIAL_DETAIL_ID == NULL)
                EscreveDisplay ("ALLOC ERROR 6");
               
        if (NR_SEQ_PESAGEM == NULL)
                EscreveDisplay ("ALLOC ERROR 7");
               
        if (CD_MATERIA_PRIMA == NULL)
                EscreveDisplay ("ALLOC ERROR 8");
               
        if (QT_REQUERIDA == NULL)
                EscreveDisplay ("ALLOC ERROR 9");
               
        if (PR_TOLERANCIA_MIN == NULL)
                EscreveDisplay ("ALLOC ERROR 10");
               
        if (PR_TOLERANCIA_MAX == NULL)
                EscreveDisplay ("ALLOC ERROR 11");
               
        if (ID_PESAR == NULL)
                EscreveDisplay ("ALLOC ERROR 12");       

}

---

---

Perhaps you can use your nice function that allocates memory and checks if it's NULL?

Code:

---

void AllocMemoria (unsigned char **pVariavel, unsigned int pTamanho){
        pVariavel = malloc (pTamanho);
 
        if (pVariavel == NULL)
                VcomSendSocket(iUserSocket, "ERRO DE ALOCAÇÃO DE MEMÓRIA", 28);       
}

---

However, you should be using *pVariavel, not pVariavel. Otherwise, you are not changing the value of the pointer outside the function.

Quote:

---

Code:

---

/*
        cSTX = 2;
        cCR  = 13;
        cLF  = 10;
        cTab = 9;
*/

#define cSTX 2
#define cCR 13
#define cLF  10
#define cTab 9

---

---

Yes.

Quote:

---

Code:

---

void ProcessaBuffer()
...
        unsigned char *cStatus;       
        unsigned char *cPeso;

---

when I Tried to alloc in this function always the application cause error, but now this function is run fine.

---

They you aren't doing it right. Use a local array - that's much easier.
Post the modified code that doesn't work.

Quote:

---

Code:

---

int ProcessaChamado():
...
        unsigned char *cFuncao;
        unsigned char *cSubFuncao;

---

same case of above

---

Same comment as above.

Quote:

---

Code:

---

void PesaItem(void):
...
                sprintf (PesoDisplay,  "%s %ld", DS_ABREVIADA, lPesoDecrescente);

---

the driver truck have at most 25000 (Kg)

---

Ok. But then why are you using a long?

Quote:

---

Code:

---

                                        strcpy (DS_ABREVIADA,cValor);

---

sorry,
this i dont undestand

---

If cValor is longer than 10 characters, it will overwrite whatever is after it. You should check that it is not longer...

I also found this:

Code:

---

                if (cStatus[0] == 'p')
                        iEstavel = 1;
                else
                        iEstavel = 0;
 
                // RETIRAR
                iEstavel = 1;

---

iEstavel is 1 at all times here.

--
Mats

ok. I will change the code, execute system, and show here the results

Code past in web page:

http://rafb.net/p/gOtSvL25.html

Complete source with headers, and turbo c project.

http://www.4shared.com/file/79687976...llosource.html


My Comments:

My AllocMemoria function is not ok, when I called it, i got for example variables with value 953614340 (lPesoDecrescente) in Function PesaItem.

with:

unsigned char *cStatus [2];
unsigned char *cPeso [7];

my system stop execution.

DS_ABREVIADA is a field of oracle database received through socket, and always have at maximum 9 chars.

But I will go test the complete source again, in nexts minutes.

Quote:

---

Originally Posted by sergioms

Code past in web page:

http://rafb.net/p/gOtSvL25.html

Complete source with headers, and turbo c project.

http://www.4shared.com/file/79687976...llosource.html


My Comments:

My AllocMemoria function is not ok, when I called it, i got for example variables with value 953614340 (lPesoDecrescente) in Function PesaItem.

---

Eh? You mean that it's not zero after allocating - no I don't expect it to be.

Quote:

---

with:

unsigned char *cStatus [2];
unsigned char *cPeso [7];
my system stop execution.

---

Drop the * - you want an array of chars, not an array of char pointers.

Quote:

---

DS_ABREVIADA is a field of oracle database received through socket, and always have at maximum 9 chars.

---

But still, a check will not hurt, right?

Also:

Code:

---

                if (iTolerancia == 1)
                        sprintf (PesoDisplay,  "RETIRE %ld", DS_ABREVIADA, lPesoDecrescente);

---

Ehm, you are printing a string as a number - that's not going to work. That probably where your large number comes from.

Pay attention to the arguments in printf.

--
Mats

Quote:

---

Originally Posted by matsp

Also:

Code:

---

                if (iTolerancia == 1)
                        sprintf (PesoDisplay,  "RETIRE %ld", DS_ABREVIADA, lPesoDecrescente);

---

Ehm, you are printing a string as a number - that's not going to work. That probably where your large number comes from.

Pay attention to the arguments in printf.

--
Mats

---

great, this solve big part of problem.
Now I Will test another system (Socket Listen (C#))
tomorrow im post news of this program.

thanks a lot
again and again
u is the guy!

Code: http://rafb.net/p/aPXhrQ59.html

I Have commented this line: //EscreveDisplay ("ORDEM CONCLUIDA", 3);
And now program continue your flow with sucessfull

I dont undestand why....

Code:

---

else if (strcmp (cFuncao, "FI") == 0){                                                        // Recebe Finaliza Item Indicando que Recebeu Todos Itens da OP
                                        iStepInput0 = 0;
                                        iItemID = 0;
                                        iPesaItem = 0;
                                        iSocketRx =0;
                                        EnviaPedidoPC ("OPFI", 4);
                                        //EscreveDisplay ("ORDEM CONCLUIDA", 3);
                                        //LimpaMemoria ();
                                        //IniciaMemoria ();

---

then program wait for VerificaBotoes called (Verified at 200 ms by function timer UserCount)
then if button 1 is pressioned call function AcaoBotaoConfirma
then iStepInput0 = 1 and call function VerificaBalancaVazia what return always 1
then now I request new call to start process EnviaPedidoPC ("OPSL", 4);
And Receive response OP + Date, received by VcomCmdUser and processed by ProcessaChamado

Code:

---

substring(&cPacote[0], cFuncao, 2);
                       
                        if (strcmp (cFuncao, "OP") == 0){                                                                // Recebe Ordem de Pesagem
                                substring(&cPacote[3], cSubFuncao, iPosTam - 2);
                                OrdemProducao (cSubFuncao);
                        }

---

and continue.....
again and again...

last code: http://pastebin.com/m796dc78a

I continue execute a program by a loop,
when next loop is call, the program crash.

I have eliminated various global variables strings, and tried your tips, but some not function perfectly with my equipment.

Any chance you could post a few samples of input?

I would rewrite the serial comms to read from a file.

--
Mats

This is a Code of Serial packets, with value weights captureds:

http://www.4shared.com/file/80304658...edoSample.html

This is a Log of my Socket Client, with all packets receiveds and send, this log is a loop complete of my program.

http://www.4shared.com/file/80304999...172161520.html

I Suspect a problem with function EscreveDisplay,

because I make a test,

I StartProgram, and next connected with my client socket app,
the my program call a function PortUserStart
then, I send to pc the number iUserSocket through EnviaPedidoPC
And my client socket has received only ...........

The function EscreveDisplay complete a text with ... to c omplete 40 chars.

Code:

---

void PortUserStart(int skt)
{
        iUserSocket = skt;
}

void EnviaPedidoPC (unsigned char *pChar, int pTamanho){
       
        unsigned char *cMsgFinal;
        int iLen;
       
        Delay (200);
       
        pTamanho += 4;
        cMsgFinal = malloc (pTamanho);
       
        if (cMsgFinal == NULL)
                VcomSendSocket(iUserSocket, "1-ERRO DE ALOCAÇÃO DE MEMÓRIA", 29);       
       
        if (iUserSocket){
                sprintf(cMsgFinal, "%c%s%c%c", cSTX, pChar, cCR, cLF);        // Formata Pacote
       
                iLen = strlen (cMsgFinal);
                VcomSendSocket(iUserSocket, cMsgFinal, iLen);       
        }
       
        free (cMsgFinal);
}

---

I know I often write a lot in my posts. And some of it may be rubbish. However:
[quote="Me"]
You are going to the 52nd element of a 51 element array - not good.

Quote:

---

Originally Posted by You

Then I Have to declare with unsigned char cDispPacote [52]; ?

---

Then Me:
Yes.

You appear to not have made this change... It will definitely not help.

--
Mats

sorry, i has change this code, but then the display dont show the message,
i was reported to, at some post back...

now i has changed again,
without display
but send, .......... continues

Code:

---

unsigned char *pMensagem

if (iLenMsg < 40){
                for(iLenCont=iLenMsg; iLenCont<40; iLenCont++)
                        strcat (pMensagem, ".");
        }

---

I suspect this part, is several to program.

Changed this:

Code:

---

unsigned char *cFuncao;
unsigned char *cSubFuncao;

---

to this:

Code:

---

unsigned char cFuncao[4];
unsigned char cSubFuncao[32];

---

The program Crash here:

Code:

---

else if (strcmp (cFuncao, "OK") == 0){                                                        // Tratamento de Confirmações
                                substring(&cPacote[2], cSubFuncao, 2);
                                if (strcmp (cSubFuncao, "OP") == 0)        {                                                // Recebe Confirmação de Ordem de Pesagem
                                        EnviaPedidoPC ("OISL", 4);               
                                }

---

in some part of this code :S

I don't see anything wrong in that code. However, it is possible that it crashes because of something else that went on BEFORE that.

--
Mats