---

Hej

Jeg er ved at lave en kø i C, hvor man får allokeret noget plads, modtager
en ticket, og senere kan man så smide dataene ind i køen. Men den virker
ikke. Det er meningen at man skal kunne hente næste element af en bestemt
slags datatype, men når jeg traverserer gennem køen, får jeg bare datatypen
"a" smidt i hovedet, ligegyldigt hvad den var i input_allocateSpace(). Jeg
har smidt to funktioner her i bunden, og i kommentarerne kan I se hvad der
er galt. Der benyttes et gennemtested linked list bibliotek. ListHead() og
ListNext() returnerer void *. Jeg er ved at gå ud af mit ellers så gode sind
(ergo: C er noget skrammel).

int input_allocateSpace (const char *datatype, unsigned long size)
{
Packet *tempPacket = malloc(sizeof(Packet));
Packet *tempPacket2 = malloc(sizeof(Packet));
int containsElements = 0;
const int limit = 10;
pthread_mutex_lock(&mutex);

// ***Snip - en masse validering***

tempPacket->datatype = (char *)malloc(sizeof(datatype));
strcpy(tempPacket->datatype, datatype);

printf("Input: input_allocateSpace() - datatype inserted: %s\n",
tempPacket->datatype);
// Her bliver datatypen skrevet rigtigt

tempPacket->ticket = (int *)malloc(sizeof(int));
*tempPacket->ticket = ticket;
ListAddHead(inQueue, tempPacket);

tempPacket2 = ListHead(inQueue);

printf("Input: input_allocateSpace() - datatype fetched: %s\n",
tempPacket2->datatype);
// Her bliver datatypen også skrevet rigtigt

if (!containsElements) {
inQueueCpy = inQueue;
one_element = 1;
} else {
one_element = 0;
}
printf("Input: input_allocateSpace() - ticket created: %i\n", ticket);

pthread_mutex_unlock(&mutex);
return ticket;
}

int input_put (const char *data, int ticket)
{
Packet *tempPacket;
pthread_mutex_lock(&mutex);

if (ticket == 0) {
pthread_mutex_unlock(&mutex);
return 0;
}

tempPacket = ListHead(inQueue);
while (tempPacket != NULL) {
if (*tempPacket->ticket == ticket) {
strcpy(tempPacket->data, data);
*tempPacket->ticket = 0;
printf("Input: input_put() - datatype: %s\n", tempPacket->datatype);
// Her er datatypen "a"???
pthread_mutex_unlock(&mutex);
return 1;
}
tempPacket = ListNext(inQueue);
}

printf("Input: input_put() - no allocated space found\n");

pthread_mutex_unlock(&mutex);
return 0;
}

--
Jakob Møbjerg Nielsen
jakob@dataloger.dk
"Hey! He reminds me of someone who looks just like him. - Me"

---

Hej Jakob

Pas på med at kalde C for noget skrammel,
nogen kunne tage det ilde op.

> int input_allocateSpace (const char *datatype, unsigned long size)
> {
> Packet *tempPacket = malloc(sizeof(Packet));
> Packet *tempPacket2 = malloc(sizeof(Packet));
> int containsElements = 0;
> const int limit = 10;
> pthread_mutex_lock(&mutex);
>
> // ***Snip - en masse validering***
>
> tempPacket->datatype = (char *)malloc(sizeof(datatype));

Er du sikker på at du får allokeret nok her?
Jeg er ikke nogen C-hacker, men giver 'sizeof(datatype)'
dig ikke bare størrelsen af en char*, og ikke længden af strengen?

> strcpy(tempPacket->datatype, datatype);
>
> printf("Input: input_allocateSpace() - datatype inserted: %s\n",
> tempPacket->datatype);
> // Her bliver datatypen skrevet rigtigt
>
> tempPacket->ticket = (int *)malloc(sizeof(int));
> *tempPacket->ticket = ticket;
> ListAddHead(inQueue, tempPacket);
>
> tempPacket2 = ListHead(inQueue);

Bibemærkning: Hvis ListHead returnerer en Packet*, er det ikke
nødvendigt at allokere tempPacket2. Den skal vel bare pege på køens
front?


/Jan

---

On 22/01/02 13.55, Jakob Møbjerg Nielsen wrote:
> Hej
>
> Jeg er ved at lave en kø i C, hvor man får allokeret noget plads, modtager
> en ticket, og senere kan man så smide dataene ind i køen. Men den virker
> ikke. Det er meningen at man skal kunne hente næste element af en bestemt
> slags datatype, men når jeg traverserer gennem køen, får jeg bare datatypen
> "a" smidt i hovedet, ligegyldigt hvad den var i input_allocateSpace(). Jeg
> har smidt to funktioner her i bunden, og i kommentarerne kan I se hvad der
> er galt. Der benyttes et gennemtested linked list bibliotek. ListHead() og
> ListNext() returnerer void *. Jeg er ved at gå ud af mit ellers så gode sind
> (ergo: C er noget skrammel).
>
> int input_allocateSpace (const char *datatype, unsigned long size)
> {
> Packet *tempPacket = malloc(sizeof(Packet));
> Packet *tempPacket2 = malloc(sizeof(Packet));
> int containsElements = 0;
> const int limit = 10;
> pthread_mutex_lock(&mutex);
>
> // ***Snip - en masse validering***
>
> tempPacket->datatype = (char *)malloc(sizeof(datatype));

proev malloc(strlen(datatype) + 1) (for nullbyte)

<SNIP>

/Daniel

--
Daniel | "Face it. You *need* some cola. It runs through your
Nielsen | blood and *sings* to you. Obtain. Open. Drink. Frolic."
| - Can of Cola (userfriendly 30.11.99)
Phone: +45 61 30 33 09

---

> proev malloc(strlen(datatype) + 1) (for nullbyte)

Det duer desværre ikke.

--
Jakob Møbjerg Nielsen
jakob@dataloger.dk
"Hey! He reminds me of someone who looks just like him. - Me"

---

On 22/01/02 16.42, Jakob Møbjerg Nielsen wrote:
> > proev malloc(strlen(datatype) + 1) (for nullbyte)
>
> Det duer desværre ikke.
>
> --
> Jakob Møbjerg Nielsen
> jakob@dataloger.dk
> "Hey! He reminds me of someone who looks just like him. - Me"
>
>
>
>

Har du proevet at compilerer noget debug ind ?
Det lyder som om du har memorycorruption et eller andet sted.

Soeg paa mpatrol! Det er et udemaerket vaerktoej.

/Daniel

--
Daniel | "Face it. You *need* some cola. It runs through your
Nielsen | blood and *sings* to you. Obtain. Open. Drink. Frolic."
| - Can of Cola (userfriendly 30.11.99)
Phone: +45 61 30 33 09

---

"Jakob Møbjerg Nielsen" <jakob@dataloger.dk> wrote:

>Hej
>
>Jeg er ved at lave en kø i C, hvor man får allokeret noget plads, modtager
>en ticket, og senere kan man så smide dataene ind i køen. Men den virker
>ikke. Det er meningen at man skal kunne hente næste element af en bestemt
>slags datatype, men når jeg traverserer gennem køen, får jeg bare datatypen
>"a" smidt i hovedet, ligegyldigt hvad den var i input_allocateSpace(). Jeg
>har smidt to funktioner her i bunden, og i kommentarerne kan I se hvad der
>er galt. Der benyttes et gennemtested linked list bibliotek. ListHead() og
>ListNext() returnerer void *. Jeg er ved at gå ud af mit ellers så gode sind
>(ergo: C er noget skrammel).
>
>int input_allocateSpace (const char *datatype, unsigned long size)
>{
> Packet *tempPacket = malloc(sizeof(Packet));
hvorfor ikke: "Packet *tempPacket = new Packet;" // ellers er det vel
rigtig nok her

> Packet *tempPacket2 = malloc(sizeof(Packet));
> int containsElements = 0;
> const int limit = 10;
> pthread_mutex_lock(&mutex);
>
> // ***Snip - en masse validering***
>
> tempPacket->datatype = (char *)malloc(sizeof(datatype));
Hvis du arbejder i et 32bit system får du allokeret 4 bytes her. Er
det rigtigt? Måske skal du bruge "strlen(datatype)+1"
Ellers kunne det være en ide at bruge en enum-type som datatype.

> strcpy(tempPacket->datatype, datatype);
>
> printf("Input: input_allocateSpace() - datatype inserted: %s\n",
>tempPacket->datatype);
> // Her bliver datatypen skrevet rigtigt
>
> tempPacket->ticket = (int *)malloc(sizeof(int));
> *tempPacket->ticket = ticket;
> ListAddHead(inQueue, tempPacket);
>
> tempPacket2 = ListHead(inQueue);
>
> printf("Input: input_allocateSpace() - datatype fetched: %s\n",
>tempPacket2->datatype);
> // Her bliver datatypen også skrevet rigtigt
>
> if (!containsElements) {
> inQueueCpy = inQueue;
> one_element = 1;
> } else {
> one_element = 0;
> }
> printf("Input: input_allocateSpace() - ticket created: %i\n", ticket);
>
> pthread_mutex_unlock(&mutex);
> return ticket;
>}
>
>int input_put (const char *data, int ticket)
>{
> Packet *tempPacket;
> pthread_mutex_lock(&mutex);
>
> if (ticket == 0) {
> pthread_mutex_unlock(&mutex);
> return 0;
> }
>
> tempPacket = ListHead(inQueue);
> while (tempPacket != NULL) {
> if (*tempPacket->ticket == ticket) {
> strcpy(tempPacket->data, data);

!!!!! du kopierer den forkerte vej
!!!!! char *strcpy( char *strDestination, const char *strSource );

> *tempPacket->ticket = 0;
> printf("Input: input_put() - datatype: %s\n", tempPacket->datatype);
> // Her er datatypen "a"???
> pthread_mutex_unlock(&mutex);
> return 1;
> }
> tempPacket = ListNext(inQueue);
> }
>
> printf("Input: input_put() - no allocated space found\n");
>
> pthread_mutex_unlock(&mutex);
> return 0;
>}

---

"Bjarne Laursen" <bl@pyramidedata.dk> wrote in message
news:hurq4ukbn3s7csd6i87d4a27fpspg1kb1i@4ax.com...
> hvorfor ikke: "Packet *tempPacket = new Packet;" // ellers er det vel
> rigtig nok her

new operatoren findes ikke i C, men kun i C++.. så det er vist ganske
korrekt.

---

> hvorfor ikke: "Packet *tempPacket = new Packet;" // ellers er det vel
> rigtig nok her

Sproget er C og Packet er en struct:

typedef struct {
char *data;
char *datatype;
int *ticket
} Packet;

> Måske skal du bruge "strlen(datatype)+1"

Det prøver jeg.

> !!!!! du kopierer den forkerte vej
> !!!!! char *strcpy( char *strDestination, const char *strSource );

Nope. Dette er en "put" funktion og ikke en "get".

--
Jakob Møbjerg Nielsen
jakob@dataloger.dk
"Hey! He reminds me of someone who looks just like him. - Me"

---

Jakob Møbjerg Nielsen <jakob@dataloger.dk> skrev:

> int input_allocateSpace (const char *datatype, unsigned long size)
> {
> Packet *tempPacket = malloc(sizeof(Packet));
> Packet *tempPacket2 = malloc(sizeof(Packet));
> int containsElements = 0;
> const int limit = 10;
> pthread_mutex_lock(&mutex);
>
> // ***Snip - en masse validering***
>
> tempPacket->datatype = (char *)malloc(sizeof(datatype));

Du bør ikke dereferere tempPacket før du har sikret dig at det er
en gyldig pointer. malloc()-kaldet ved initialiseringen kan have
fejlet.

Hvad er "datatype"? Er du sikker at du får allokeret den rigtige
mængde plads?

> strcpy(tempPacket->datatype, datatype);

Du bør ikke bruge tempPacket->datatype før du har sikret dig at det
er en gyldig pointer. malloc()-kaldet kan have fejlet.

Du risikerer bufferoverløb her. strncpy() + en eksplicit
null-terminering ville være sikrere.

> printf("Input: input_allocateSpace() - datatype inserted: %s\n",
> tempPacket->datatype);
> // Her bliver datatypen skrevet rigtigt
>
> tempPacket->ticket = (int *)malloc(sizeof(int));
> *tempPacket->ticket = ticket;

Du bør ikke bruge tempPacket->ticket før du har sikret dig at det
er en gyldig pointer. malloc()-kaldet kan have fejlet.

Hvad er "ticket" som står på højre side af lighedstegnet?

> ListAddHead(inQueue, tempPacket);

Hvad er "inQueue"?

Processing stopped! Too many unknown external references....

---

> Du bør ikke dereferere tempPacket før du har sikret dig at det er
> en gyldig pointer. malloc()-kaldet ved initialiseringen kan have
> fejlet.

???

> Hvad er "datatype"? Er du sikker at du får allokeret den rigtige
> mængde plads?

Det er en char *. Kig i argumenterne.

> Du bør ikke bruge tempPacket->datatype før du har sikret dig at det
> er en gyldig pointer. malloc()-kaldet kan have fejlet.

Hvad mener du?

> Du bør ikke bruge tempPacket->ticket før du har sikret dig at det
> er en gyldig pointer. malloc()-kaldet kan have fejlet.
>
> Hvad er "ticket" som står på højre side af lighedstegnet?

Ticket fungerer fint. Det er forøvrigt en int *.

> Hvad er "inQueue"?

En linked list.

--
Jakob Møbjerg Nielsen
jakob@dataloger.dk
"Hey! He reminds me of someone who looks just like him. - Me"

---

Jakob Møbjerg Nielsen <jakob@dataloger.dk> skrev:
>> Du bør ikke dereferere tempPacket før du har sikret dig at det er
>> en gyldig pointer. malloc()-kaldet ved initialiseringen kan have
>> fejlet.
>
> ???

Jeg mener at du initialiserer tempPacket med et malloc()-kald uden
at kontrollere hvad kaldet returnerer. malloc() kan give en
NULL-pointer.

>> Hvad er "datatype"? Er du sikker at du får allokeret den rigtige
>> mængde plads?
>
> Det er en char *. Kig i argumenterne.

Det kan jeg ikke se i argumenterne. Men det er helt galt at allokere
plads til pointer, og så bagefter kopiere en streng af uspecificret
længde ind på pladsen (hvis længden havde været specificeret, var
det også galt).

>> Du bør ikke bruge tempPacket->datatype før du har sikret dig at det
>> er en gyldig pointer. malloc()-kaldet kan have fejlet.
>
> Hvad mener du?

Det samme som ovenfor.

---

> > Det er en char *. Kig i argumenterne.
>
> Det kan jeg ikke se i argumenterne. Men det er helt galt at allokere
> plads til pointer, og så bagefter kopiere en streng af uspecificret
> længde ind på pladsen (hvis længden havde været specificeret, var
> det også galt).

Der er faktisk en const char *:

"int input_allocateSpace (const char *datatype, unsigned long size)"
^^^^^^^^^^^^^^^^^^^^

--
Jakob Møbjerg Nielsen
jakob@dataloger.dk
"Hey! He reminds me of someone who looks just like him. - Me"

---

Jakob Møbjerg Nielsen <jakob@dataloger.dk> skrev:
>> > Det er en char *. Kig i argumenterne.
>>
>> Det kan jeg ikke se i argumenterne. Men det er helt galt at allokere
>> plads til pointer, og så bagefter kopiere en streng af uspecificret
>> længde ind på pladsen (hvis længden havde været specificeret, var
>> det også galt).
>
> Der er faktisk en const char *:
>
> "int input_allocateSpace (const char *datatype, unsigned long size)"
> ^^^^^^^^^^^^^^^^^^^^

Minsandten ja. Så skal du ikke reservere den mængde plads som
sådan en pointer i sig selv fylder, men den mængde plads som de
data som den peger på, fylder, før du kopierer dataene.

---

"Jakob Møbjerg Nielsen" <jakob@dataloger.dk> skrev:
>int input_allocateSpace (const char *datatype, unsigned long size)
>....
> tempPacket->datatype = (char *)malloc(sizeof(datatype));
> strcpy(tempPacket->datatype, datatype);
> ...
> if (*tempPacket->ticket == ticket) {
> strcpy(tempPacket->data, data);

Dine strcpy'er er klassisk forkert.

Først allokerer malloc(sizeof(const char*)) plads til en pointer, dvs
typisk 4 bytes, og derefter kopieres hele indholdet af strengen datatype
ned i de sølle 4 bytes. Med mindre alle strenge er på højst 3 tegn vil
det hurtigt gå rivende galt.

Du skal derfor enten:

1) Allokere plads til en hel kopi af strengen, dvs malloc(strlen(...)+1)
(Og pas så på med strcpy() nr 2, for der skal t->data reallokeres for
at sikre, at den er stor nok til den nye data)
2) Altid kun jonglere med pointere til den oprindelige streng.

Vælger du 2 skal dine strcpy(tempPacket->data, x) bl.a. udskiftes til
tempPacket->data = x, dvs et simpelt pointer-assignment.

--
Richard Flamsholt
richard@flamsholt.dk - www.richard.flamsholt.dk

---

Nu virker det:

> tempPacket = ListHead(inQueue);
> while (tempPacket != NULL) {
> if (*tempPacket->ticket == ticket) {
//Her skulle der være en realloc()... Det er grimt,
//men det virker, og skal ikke bruges til andet end
//at demonstrere et design til en eksamen.


--
Jakob Møbjerg Nielsen
jakob@dataloger.dk
"Hey! He reminds me of someone who looks just like him. - Me"

---

"Jakob Møbjerg Nielsen" <jakob@dataloger.dk> skrev:
>Nu virker det:

Ikke hvis du kun har ændret det her:

>> while (tempPacket != NULL) {
>> if (*tempPacket->ticket == ticket) {
> //Her skulle der være en realloc()...

- og ikke også har fixet den her fejl:

>>> tempPacket->datatype = (char *)malloc(sizeof(datatype));
>>> strcpy(tempPacket->datatype, datatype);

Som efterhånden mange har fortalt dig er malloc(strlen(datatype)+1) det
rigtige at bruge. Ovenstående er en fejl, der afsløres hvis strengen er
på mere end 3 tegn.

--
Richard Flamsholt
richard@flamsholt.dk - www.richard.flamsholt.dk

---

Richard Flamsholt <richard@flamsholt.dk> skrev:
> Som efterhånden mange har fortalt dig er malloc(strlen(datatype)+1) det
> rigtige at bruge. Ovenstående er en fejl, der afsløres hvis strengen er
> på mere end 3 tegn.

Eller mere end 7 tegn hvis sizeof(char *) er 8, eller ...

---

Byrial Jensen <bjensen@nospam.dk> skrev:
>Eller mere end 7 tegn hvis sizeof(char *) er 8, eller ...

Jep. Jeg skrev også "typisk 4 bytes" i det første indlæg, men valgte den
korte og kontante måde her for at undgå at drukne budskabet i detaljer.

--
Richard Flamsholt
richard@flamsholt.dk - www.richard.flamsholt.dk

---

> Som efterhånden mange har fortalt dig er malloc(strlen(datatype)+1)
det
> rigtige at bruge. Ovenstående er en fejl, der afsløres hvis strengen
er
> på mere end 3 tegn.

Nå ja... det har jeg også fixet.

--
Jakob Møbjerg Nielsen
jakob@dataloger.dk
"Hey! He reminds me of someone who looks just like him. - Me"