Best method to alloc memory for string and desalloc.

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