fifo algotithm

[cerr]

Hi There,

I'm trying to write some kind of fifo algorithm.
I have a global pointer "char MyQueue[1024];" and try to append a new
string on the beginning by shifting the current content to the right.
I tried doing so this way:

  char* tmp;
  // Verifying data length is set > 0
  if (newDataLength<=0)
    {
    printf("Error: data length is 0\n");
    return 1;
    }
  //checking if newData is unqual 0
  if (strlen(newData)<0)
    {
    printf("no content to add to Fifo-Queue\n: %d bytes",strlen
(newData));
    return 1;
    }
  // calculating queue new length
  int calclen=newDataLength+strlen(MyQueue);
  printf("newDataLength:%d\nstrlen(MyQueue):%d\ncalclen:%d
\n",newDataLength,strlen(MyQueue),calclen);
  //appending new string to current queue
  char* catstr=strncat(newData, (const char*)MyQueue, strlen
(MyQueue));//PROBLEM is here!
  printf("catstr:%s\n",catstr);
  if (strlen(MyQueue)>0)
    tmp=MyQueue;
  memmove(MyQueue, catstr, calclen);
  return 0;
}

But when I call strncat I get a segmentation fault and i'm not sure
why, newData exists, the MyQueue pointer exists...Why do I get a
segmentation fault?
Thanks for suggestions and hints!

 

[Eric Sosman]

Hi There,

I'm trying to write some kind of fifo algorithm.
I have a global pointer "char MyQueue[1024];" and try to append a new
string on the beginning by shifting the current content to the right.
I tried doing so this way:

[...]
char* catstr=strncat(newData, (const char*)MyQueue, strlen
(MyQueue));//PROBLEM is here!
[...]

But when I call strncat I get a segmentation fault and i'm not sure
why, newData exists, the MyQueue pointer exists...Why do I get a
segmentation fault?
Thanks for suggestions and hints!

Are you sure that newData points to a modifiable
string that has enough room for calclen additional
characters?

If that doesn't help you, please post your code. No,
*not* the incomplete fragment you've given here, but a
complete, compilable program that runs and crashes.

 

[Barry Schwarz]

Hi There,

I'm trying to write some kind of fifo algorithm.
I have a global pointer "char MyQueue[1024];" and try to append a new

You do not have pointer. You have an array.

On the other hand, the return statements in your code implies this is
extracted from a function (it would have been nice to see the whole
function) so I will assume newData is a parameter and therefore a
pointer.

string on the beginning by shifting the current content to the right.
I tried doing so this way:

char* tmp;
// Verifying data length is set > 0
if (newDataLength<=0)
{
printf("Error: data length is 0\n");
return 1;
}
//checking if newData is unqual 0
if (strlen(newData)<0)[/QUOTE]

strlen returns a size_t which is unsigned and can therefore never be
less than 0.  On the other hand, the string "" has a length of 0 as
does the string "\0"; so you probably want to replace < with ==.
[QUOTE]
{
printf("no content to add to Fifo-Queue\n: %d bytes",strlen
(newData));
return 1;
}
// calculating queue new length[/QUOTE]

If you want help from people who don't have a C99 compiler, use the
standard /* ... */ comment syntax.
[QUOTE]
int calclen=newDataLength+strlen(MyQueue);[/QUOTE]

At this point it would be appropriate to insure that 
calclen+1 < sizeof MyQueue.
[QUOTE]
printf("newDataLength:%d\nstrlen(MyQueue):%d\ncalclen:%d
\n",newDataLength,strlen(MyQueue),calclen);[/QUOTE]

Horizontal white space is free.  Judicious spacing can make your code
and your output a lot more readable.

Problem 1: The expression strlen(MyQueue) evaluates to a size_t which
is definitely unsigned but need not be int.  You try to print it with
%d which requires a signed int, resulting in undefined behavior.  You
could cast the expression to int since in this case it cannot evaluate
to a value greater than 1023.  But the usual recommendation is to cast
it to unsigned long and change the format to %ul.
[QUOTE]
//appending new string to current queue
char* catstr=strncat(newData, (const char*)MyQueue, strlen
(MyQueue));//PROBLEM is here![/QUOTE]

strncat will return newData.  You now have two pointer expressions
that point to the same char.  You never modify catstr again so it is
redundant.

Problem 2: Before C99, declarations must precede executable
statements.

Problem 3: This attempts to place all the data from MyQueue at the end
of the current string pointed to by newData.  Are you absolutely
CERTAIN it will always have enough room?  Can you even be certain that
newData points to modifiable data (string literals can be function
arguments)?
[QUOTE]
printf("catstr:%s\n",catstr);
if (strlen(MyQueue)>0)
tmp=MyQueue;[/QUOTE]

What is this supposed to accomplish?
[QUOTE]
memmove(MyQueue, catstr, calclen);[/QUOTE]

You don't need the potential additional overhead associated with
memmove when memcpy will achieve the same result.

Problem 4: You do not move (or copy) the terminating '\0'
[QUOTE]
return 0;
}

But when I call strncat I get a segmentation fault and i'm not sure
why, newData exists, the MyQueue pointer exists...Why do I get a

It is not sufficient to know that newData exists. We need to know
what it points to. You need to provide the complete function and a
sample main() which calls the function and exhibits the problem.

segmentation fault?

Possibilities:

newData points to a string literal.
Resulting "string" is larger than MyQueue.
newData points to an array whose size is less than calclen+1
size_t is unsigned long and printf invokes undefined behavior
that doesn't manifest itself till later.
There is a problem in the code you haven't shown us.

Just out of curiosity, consider the following algorithm:
Perform the appropriate checks on MyQueue and newData.
Use memmove to shift the bytes in MyQueue to the right.
Use memcpy to copy the bytes from newData to the beginning of
MyQueue.

 

[cerr]

Hi There,

I'm trying to write some kind of fifo algorithm.
I have a global pointer "char MyQueue[1024];" and try to append a new

You do not have pointer.  You have an array.

Yup, I actually wanted it to be a pointer so that I can copy "endless"
data into it.
But I just didn't get it going... - Can you help me if i declared
it char* MyQueue?

On the other hand, the return statements in your code implies this is
extracted from a function (it would have been nice to see the whole
function) so I will assume newData is a parameter and therefore a
pointer.

string on the beginning by shifting the current content to the right.
I tried doing so this way:

 char* tmp;
 // Verifying data length is set > 0
 if (newDataLength<=0)
   {
   printf("Error: data length is 0\n");
   return 1;
   }
 //checking if newData is unqual 0
 if (strlen(newData)<0)[/QUOTE]

strlen returns a size_t which is unsigned and can therefore never be
less than 0.  On the other hand, the string "" has a length of 0 as
does the string "\0"; so you probably want to replace < with ==.
[QUOTE]
   {
   printf("no content to add to Fifo-Queue\n: %d bytes",strlen
(newData));
   return 1;
   }
 // calculating queue new length[/QUOTE]

If you want help from people who don't have a C99 compiler, use the
standard /* ... */ comment syntax.
[QUOTE]
 int calclen=newDataLength+strlen(MyQueue);[/QUOTE]

At this point it would be appropriate to insure that
calclen+1 < sizeof MyQueue.
[QUOTE]
 printf("newDataLength:%d\nstrlen(MyQueue):%d\ncalclen:%d
\n",newDataLength,strlen(MyQueue),calclen);[/QUOTE]

Horizontal white space is free.  Judicious spacing can make your code
and your output a lot more readable.

Problem 1: The expression strlen(MyQueue) evaluates to a size_t which
is definitely unsigned but need not be int.  You try to print it with
%d which requires a signed int, resulting in undefined behavior.  You
could cast the expression to int since in this case it cannot evaluate
to a value greater than 1023.  But the usual recommendation is to cast
it to unsigned long and change the format to %ul.
[QUOTE]
 //appending new string to current queue
 char* catstr=strncat(newData, (const char*)MyQueue, strlen
(MyQueue));//PROBLEM is here![/QUOTE]

strncat will return newData.  You now have two pointer expressions
that point to the same char.  You never modify catstr again so it is
redundant.

Problem 2: Before C99, declarations must precede executable
statements.

Problem 3: This attempts to place all the data from MyQueue at the end
of the current string pointed to by newData.  Are you absolutely
CERTAIN it will always have enough room?  Can you even be certain that
newData points to modifiable data (string literals can be function
arguments)?
[QUOTE]
 printf("catstr:%s\n",catstr);
 if (strlen(MyQueue)>0)
   tmp=MyQueue;[/QUOTE]

What is this supposed to accomplish?
[QUOTE]
 memmove(MyQueue, catstr, calclen);[/QUOTE]

You don't need the potential additional overhead associated with
memmove when memcpy will achieve the same result.

Problem 4: You do not move (or copy) the terminating '\0'
[QUOTE]
 return 0;
}

But when I call strncat I get a segmentation fault and i'm not sure
why, newData exists, the MyQueue pointer exists...Why do I get a

It is not sufficient to know that newData exists.  We need to know
what it points to.  You need to provide the complete function and a
sample main() which calls the function and exhibits the problem.

segmentation fault?

Possibilities:

        newData points to a string literal.
        Resulting "string" is larger than MyQueue.
        newData points to an array whose size is less than calclen+1
        size_t is unsigned long and printf invokes undefined behavior
that doesn't manifest itself till later.
        There is a problem in the code you haven't shown us.

Just out of curiosity, consider the following algorithm:
        Perform the appropriate checks on MyQueue and newData.
        Use memmove to shift the bytes in MyQueue to the right.
        Use memcpy to copy the bytes from newData to the beginning of
MyQueue.

I tried to correct most of the things but still couldn't get it
going...
I still would like to have an "endless" queue pointer and apparently
get rid of the segmentation fault that I'm getting compiling it with
gcc 4.3.2.
My whole code including main looks like:

#include <stdio.h>
#include <pthread.h>
#include <string.h>
//bool AppendToFIFO( t_FIFO* FIFO, void* newData, int newDataLength);
// global declarations
char MyQueue[1024];
int AppendToFIFO(void* newData, int newDataLength);

int main (int argc, char* argv[])
{
  //setting the queue's content to 0
  memset(&MyQueue,'\0', 1024);
  printf("memset MyQueue:0x%s\n",MyQueue);
  char* str1="TheseAre16chars";
//appending data loaded in number to the queue
  if(AppendToFIFO(str1, 16)==0)
  {
    printf("1st Conent of Queue:%s\n",MyQueue);
  }
else
  printf("Problem appending data to Queue\n");

  char* str2="hello";
//appending data loaded in number to the queue
  if(AppendToFIFO(str2, 5)==0)
  {
    printf("2nd Conent of Queue:%s\n",MyQueue);
  }
else
  printf("Problem appending data to Queue\n");

  return 0;
}

int AppendToFIFO(void* newData, int newDataLength)
{
/
***********************************************************************************
function: AppendToFIFO
          appending newDataLength bytes from newData to MyQueue.
return value: 0 on success
              1 on failure
attributes: void* newData - pointer to data to be added to queue
            int newDataLength - number of bytes from newData to be
added to MyQueue
***********************************************************************************/
  char* tmp;
  /* Verifying data length is set > 0*/
  if (newDataLength<=0)
    {
    printf("Error: data length is 0\n");
    return 1;
    }
  /*checking if newData is unqual 0*/
  if (strlen(newData)==0)
    {
    printf("no content to add to Fifo-Queue\n: %d bytes",strlen
(newData));
    return 1;
    }
  /* calculating queue new length*/
  int calclen=newDataLength+strlen(MyQueue);
  printf("newDataLength:%d\nstrlen(MyQueue):%d\ncalclen:%d
\n",newDataLength,strlen(MyQueue),calclen);
  /*appending new string to current queue*/
strncat(newData, (const char*)MyQueue, strlen(MyQueue));/*PROBLEM is
here!*/
  printf("newData:%s\n",(char *)newData);
  if (strlen(MyQueue)>0)
    tmp=MyQueue;
  memcpy(MyQueue, newData, calclen);
  return 0;
}

Thank you!
Ron

 

[Ben Bacarisse]

I'm trying to write some kind of fifo algorithm.
I have a global pointer "char MyQueue[1024];" and try to append a new

You do not have pointer.  You have an array.

Yup, I actually wanted it to be a pointer so that I can copy
"endless" data into it. But I just didn't get it going... - Can
you help me if i declared it char* MyQueue?

If you wanted to go that route, one way is to use a buffer that is
allocated and grown as needed (look up malloc and realloc in your C
reference of choice). It is not an efficient way to do it but it
might be quite fast enough.

I tried to correct most of the things but still couldn't get it
going...

You've missed a vital comment. Rather than comment on all the things
I could comment on I'll cut to the chase...

strncat(newData, (const char*)MyQueue, strlen(MyQueue));

You can't do this. The string pointed to by newData is not modifiable
an even if it were there is no reason to suppose that there is room
for the content of MyQueue after it.

memcpy(MyQueue, newData, calclen);

The whole idea looks backwards. To add "x" to a queue that has "abc"
in it already I just need to strcat(queue, "x"). You seem to be
putting the new stuff at the front.

 

[cerr]

I'm trying to write some kind of fifo algorithm.
I have a global pointer "char MyQueue[1024];" and try to append a new
You do not have pointer.  You have an array.

Yup, I actually wanted it to be a pointer so that I can copy
"endless" data into it.  But I just didn't get it going... - Can
you help me if i declared it char* MyQueue?

If you wanted to go that route, one way is to use a buffer that is
allocated and grown as needed (look up malloc and realloc in your C
reference of choice).  It is not an efficient way to do it but it
might be quite fast enough.

I tried to correct most of the things but still couldn't get it
going...

You've missed a vital comment.  Rather than comment on all the things
I could comment on I'll cut to the chase...

strncat(newData, (const char*)MyQueue, strlen(MyQueue));

You can't do this.  The string pointed to by newData is not modifiable
an even if it were there is no reason to suppose that there is room
for the content of MyQueue after it.

  memcpy(MyQueue, newData, calclen);

The whole idea looks backwards.  To add "x" to a queue that has "abc"
in it already I just need to strcat(queue, "x").  You seem to be
putting the new stuff at the front.

But in a queue new content should be added left aligned because it's a
fifo( first in, first out) system - no?
am i understanding it wrongly?
Okay, I tried it again, this time with realloc():
/*****
SOME HEAD
fifo.c
****/

#include <stdio.h>
#include <pthread.h>
#include <string.h>
int AppendToFIFO(char *locQueue, void* newData, int newDataLength);

int main (int argc, char* argv[])
{
char *MyQueue;

char* str1="TheseAre16chars";
/*appending data to the queue*/
if(AppendToFIFO(MyQueue, str1, 16)==0)
{
printf("1st Conent of Queue:%s\n",MyQueue);
}
else
printf("Problem appending data to Queue\n");

char* str2="hello";
/*appending data loaded in number to the queue*/
if(AppendToFIFO(MyQueue, str2, 5)==0)
{
printf("2nd Conent of Queue:%s\n",MyQueue);
}
else
printf("Problem appending data to Queue\n");

return 0;
}

int AppendToFIFO(char* locQueue, void* newData, int newDataLength)
{
/
***********************************************************************************
function: AppendToFIFO
appending newDataLength bytes from newData to MyQueue.
return value: 0 on success
1 on failure
attributes: void* newData - pointer to data to be added to queue
int newDataLength - number of bytes from newData to be
added to MyQueue
***********************************************************************************/
char* tmp;
/* Verifying data length is set > 0*/
if (newDataLength<=0)
{
printf("Error: data length is 0\n");
return 1;
}
/*checking if newData is unqual 0*/
if (strlen(newData)==0)
{
printf("no content to add to Fifo-Queue\n: %d bytes",strlen
(newData));
return 1;
}
printf("strlen(locQueue):%d\n",strlen(locQueue));
/*allocating new memory size */
*locQueue = realloc(*locQueue, (strlen(locQueue) + newDataLength));
tmp=locQueue;
/* How can i add the new content aligned to the left withou a
atrcat? strcat somehow doesn't seem the right solution for this, does
it? */
strcat(newData, (const char*)locQueue);
/* copying new string into allocated memory */
memcpy(locQueue, newData, strlen(locQueue));
return 0;
}
Not sure if here's another possibility than using strcat/strncat in
order to add the content left aligned - but anyways, now i get a seg
fault at my realloc call even tho strlen(locQueue) returns 9....#1
bwhy does it return 9 when it's empty, #2 why do i keep getting a seg
fault?

Thanks!

 

[Keith Thompson]

Okay, I tried it again, this time with realloc():

I haven't studied the entire thing, but a few things jumped out at me.

/*****
SOME HEAD
fifo.c
****/

#include <stdio.h>
#include <pthread.h>

This header is non-standard. I don't think you're using it in your
posted code (though you may be using it in the full version).

#include <string.h> [...]

int AppendToFIFO(char* locQueue, void* newData, int newDataLength)
{ [...]
/
***********************************************************************************
function: AppendToFIFO
appending newDataLength bytes from newData to MyQueue.
return value: 0 on success
1 on failure
attributes: void* newData - pointer to data to be added to queue
int newDataLength - number of bytes from newData to be
added to MyQueue
***********************************************************************************/
char* tmp;
/* Verifying data length is set > 0*/
if (newDataLength<=0)
{
printf("Error: data length is 0\n");
return 1;
}

Normally a function should return an error indication and let the
caller decided how to deal with it. Printing an error message within
a function like this is ok during debugging, but a bad idea in
production code.

[...]

printf("strlen(locQueue):%d\n",strlen(locQueue));

strlen() returns a result of type size_t; "%d" expects an argument of
type int. Try this:

printf("strlen(locQueue):%zu\n", strlen(locQueue));

or, if your printf doesn't support "%zu" (it's new in C99):

printf("strlen(locQueue):%lu\n", (unsigned long)strlen(locQueue));

/*allocating new memory size */
*locQueue = realloc(*locQueue, (strlen(locQueue) + newDataLength));

Since locQueue is of type char*, *locQueue is of type char. Do you
really want to assign the void* value returned by realloc() to a char
object?

realloc is declared in <stdlib.h>, but I don't see a "#include <stdlib.h>".

Your compiler should have warned you about this. If it doesn't, crank
up the warning level.

tmp=locQueue;
/* How can i add the new content aligned to the left withou a
atrcat? strcat somehow doesn't seem the right solution for this, does
it? */
strcat(newData, (const char*)locQueue);

There's no need for the cast.

[...]

Not sure if here's another possibility than using strcat/strncat in

[...]

 

[cerr]

Hi Keith,

Thanks for your help!

[...]

Okay, I tried it again, this time with realloc():

I haven't studied the entire thing, but a few things jumped out at me.

/*****
SOME HEAD
fifo.c
****/

#include <stdio.h>
#include <pthread.h>

This header is non-standard.  I don't think you're using it in your
posted code (though you may be using it in the full version).

right, I tried something earlier and forgot to take it back out -
thanks

#include <string.h>
[...]





int AppendToFIFO(char* locQueue, void* newData, int newDataLength)
{ [...]
/
***********************************************************************************
function: AppendToFIFO
          appending newDataLength bytes from newData to MyQueue.
return value: 0 on success
              1 on failure
attributes: void* newData - pointer to data to be added to queue
            int newDataLength - number of bytes from newData to be
added to MyQueue
***********************************************************************************/
  char* tmp;
  /* Verifying data length is set > 0*/
  if (newDataLength<=0)
    {
    printf("Error: data length is 0\n");
    return 1;
    }

Normally a function should return an error indication and let the
caller decided how to deal with it.  Printing an error message within
a function like this is ok during debugging, but a bad idea in
production code.

Yup, I'm aware of this but this is code in development and things
gotta go quickly sometimes

[...]

  printf("strlen(locQueue):%d\n",strlen(locQueue));

strlen() returns a result of type size_t; "%d" expects an argument of
type int.  Try this:

printf("strlen(locQueue):%zu\n", strlen(locQueue));

or, if your printf doesn't support "%zu" (it's new in C99):

printf("strlen(locQueue):%lu\n", (unsigned long)strlen(locQueue));

  /*allocating new memory size */
  *locQueue = realloc(*locQueue, (strlen(locQueue) + newDataLength));

Since locQueue is of type char*, *locQueue is of type char.  Do you
really want to assign the void* value returned by realloc() to a char
object?

Oh yeah, you're right...hoops...

realloc is declared in <stdlib.h>, but I don't see a "#include <stdlib.h>".

Your compiler should have warned you about this.  If it doesn't, crank
up the warning level.

gcc actually did give me a warning but i just ignored it....added
stdlib now

  tmp=locQueue;
  /* How can i add the new content aligned to the left withou a
atrcat? strcat somehow doesn't seem the right solution for this, does
it? */
  strcat(newData, (const char*)locQueue);

There's no need for the cast.

[...]

Not sure if here's another possibility than using strcat/strncat in

[...]

--
Keith Thompson (The_Other_Keith) (e-mail address removed)  <http://www.ghoti.net/~kst>
Nokia
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"

Ok and after applying these changes my code looks like this:

/*****
SOME HEAD
fifo.c
****/
#include <stdlib.h>
#include <stdio.h>
#include <string.h>

int AppendToFIFO(char *locQueue, void* newData, int newDataLength);

int main (int argc, char* argv[])
{
char *MyQueue;

char* str1="TheseAre16chars";
/*appending data to the queue*/
if(AppendToFIFO(MyQueue, str1, 16)==0)
{
printf("1st Conent of Queue:%s\n",MyQueue);
}
else
printf("Problem appending data to Queue\n");

char* str2="hello";
/*appending data loaded in number to the queue*/
if(AppendToFIFO(MyQueue, str2, 5)==0)
{
printf("2nd Conent of Queue:%s\n",MyQueue);
}
else
printf("Problem appending data to Queue\n");

return 0;
}

int AppendToFIFO(char* locQueue, void* newData, int newDataLength)
{
/
***********************************************************************************
function: AppendToFIFO
appending newDataLength bytes from newData to MyQueue.
return value: 0 on success
!0 on failure
attributes: void* newData - pointer to data to be added to queue
int newDataLength - number of bytes from newData to be
added to MyQueue
return values:1 newDataLength is equal or smaller 0
2 length is newData is 0
***********************************************************************************/
char* tmp;
/* Verifying data length is set > 0*/
if (newDataLength<=0)
{
return 1;
}
/*checking if newData is unqual 0*/
if (strlen(newData)==0)
{
return 2;
}
printf("strlen(locQueue):%zu\n",strlen(locQueue));
/*allocating new memory size */
locQueue = realloc(locQueue, (strlen(locQueue) + newDataLength));
tmp=locQueue;
/* How can i add the new content aligned to the left withou a
atrcat? strcat somehow doesn't seem the right solution for this, does
it? */
strcat(newData, locQueue);
/* copying new string into allocated memory */
memcpy(locQueue, newData, strlen(locQueue));
return 0;
}

and i get all weird terminal output, look at this:
reg@reg-laptop:~$ gcc -o fifo fifo.c
reg@reg-laptop:~$ ./fifo
strlen(locQueue):9
*** glibc detected *** ./fifo: realloc(): invalid pointer: 0xb7faff50
***
======= Backtrace: =========
/lib/tls/i686/cmov/libc.so.6[0xb7e9d3f4]
/lib/tls/i686/cmov/libc.so.6(realloc+0x242)[0xb7ea1ec2]
/lib/tls/i686/cmov/libc.so.6(realloc+0x42)[0xb7ea1cc2]
../fifo[0x80485da]
../fifo[0x80484f6]
/lib/tls/i686/cmov/libc.so.6(__libc_start_main+0xe5)[0xb7e44685]
../fifo[0x8048431]
======= Memory map: ========
08048000-08049000 r-xp 00000000 08:03 408296 /home/reg/fifo
08049000-0804a000 r--p 00000000 08:03 408296 /home/reg/fifo
0804a000-0804b000 rw-p 00001000 08:03 408296 /home/reg/fifo
09c42000-09c63000 rw-p 09c42000 00:00 0 [heap]
b7e2d000-b7e2e000 rw-p b7e2d000 00:00 0
b7e2e000-b7f86000 r-xp 00000000 08:01 1133723 /lib/tls/i686/cmov/
libc-2.8.90.so
b7f86000-b7f88000 r--p 00158000 08:01 1133723 /lib/tls/i686/cmov/
libc-2.8.90.so
b7f88000-b7f89000 rw-p 0015a000 08:01 1133723 /lib/tls/i686/cmov/
libc-2.8.90.so
b7f89000-b7f8c000 rw-p b7f89000 00:00 0
b7f90000-b7f9d000 r-xp 00000000 08:01 1116350 /lib/libgcc_s.so.1
b7f9d000-b7f9e000 r--p 0000c000 08:01 1116350 /lib/libgcc_s.so.1
b7f9e000-b7f9f000 rw-p 0000d000 08:01 1116350 /lib/libgcc_s.so.1
b7f9f000-b7fa2000 rw-p b7f9f000 00:00 0
b7fa2000-b7fbc000 r-xp 00000000 08:01 1116307 /lib/ld-2.8.90.so
b7fbc000-b7fbd000 r-xp b7fbc000 00:00 0 [vdso]
b7fbd000-b7fbe000 r--p 0001a000 08:01 1116307 /lib/ld-2.8.90.so
b7fbe000-b7fbf000 rw-p 0001b000 08:01 1116307 /lib/ld-2.8.90.so
bfeaa000-bfebf000 rw-p bffeb000 00:00 0 [stack]
Aborted
reg@reg-laptop:~$

I'm not sure why it dumps the memory map tho...

 

[cerr]

Hi Keith,

Thanks for your help!

[...]

Okay, I tried it again, this time with realloc():

I haven't studied the entire thing, but a few things jumped out at me.

This header is non-standard.  I don't think you're using it in your
posted code (though you may be using it in the full version).

right, I tried something earlier and forgot to take it back out -
thanks

#include <string.h>

[...]

int AppendToFIFO(char* locQueue, void* newData, int newDataLength)
{ [...]
/
***********************************************************************************
function: AppendToFIFO
          appending newDataLength bytes from newData to MyQueue.
return value: 0 on success
              1 on failure
attributes: void* newData - pointer to data to be added to queue
            int newDataLength - number of bytes from newData to be
added to MyQueue
***********************************************************************************/
  char* tmp;
  /* Verifying data length is set > 0*/
  if (newDataLength<=0)
    {
    printf("Error: data length is 0\n");
    return 1;
    }

Normally a function should return an error indication and let the
caller decided how to deal with it.  Printing an error message within
a function like this is ok during debugging, but a bad idea in
production code.

Yup, I'm aware of this but this is code in development and things
gotta go quickly sometimes

[...]

  printf("strlen(locQueue):%d\n",strlen(locQueue));

strlen() returns a result of type size_t; "%d" expects an argument of
type int.  Try this:

printf("strlen(locQueue):%zu\n", strlen(locQueue));

or, if your printf doesn't support "%zu" (it's new in C99):

printf("strlen(locQueue):%lu\n", (unsigned long)strlen(locQueue));

Since locQueue is of type char*, *locQueue is of type char.  Do you
really want to assign the void* value returned by realloc() to a char
object?

Oh yeah, you're right...hoops...

realloc is declared in <stdlib.h>, but I don't see a "#include <stdlib.h>".

Your compiler should have warned you about this.  If it doesn't, crank
up the warning level.

gcc actually did give me a warning but i just ignored it....added
stdlib now

There's no need for the cast.

[...]

Not sure if here's another possibility than using strcat/strncat in

[...]

--
Keith Thompson (The_Other_Keith) (e-mail address removed)  <http://www.ghoti.net/~kst>
Nokia
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"

Ok and after applying these changes my code looks like this:

/*****
SOME HEAD
fifo.c
****/
#include <stdlib.h>
#include <stdio.h>
#include <string.h>

int AppendToFIFO(char *locQueue, void* newData, int newDataLength);

int main (int argc, char* argv[])
{
  char *MyQueue;

  char* str1="TheseAre16chars";
  /*appending data to the queue*/
  if(AppendToFIFO(MyQueue, str1, 16)==0)
  {
    printf("1st Conent of Queue:%s\n",MyQueue);
  }
else
  printf("Problem appending data to Queue\n");

  char* str2="hello";
  /*appending data loaded in number to the queue*/
  if(AppendToFIFO(MyQueue, str2, 5)==0)
  {
    printf("2nd Conent of Queue:%s\n",MyQueue);
  }
else
  printf("Problem appending data to Queue\n");

  return 0;

}

int AppendToFIFO(char* locQueue, void* newData, int newDataLength)
{
/
***********************************************************************************
function: AppendToFIFO
          appending newDataLength bytes from newData to MyQueue..
return value: 0 on success
              !0 on failure
attributes: void* newData - pointer to data to be added to queue
            int newDataLength - number of bytes from newData to be
added to MyQueue
return values:1 newDataLength is equal or smaller 0
              2 length is newData is 0
***********************************************************************************/
  char* tmp;
  /* Verifying data length is set > 0*/
  if (newDataLength<=0)
    {
    return 1;
    }
  /*checking if newData is unqual 0*/
  if (strlen(newData)==0)
    {
    return 2;
    }
  printf("strlen(locQueue):%zu\n",strlen(locQueue));
  /*allocating new memory size */
  locQueue = realloc(locQueue, (strlen(locQueue) + newDataLength));
  tmp=locQueue;
  /* How can i add the new content aligned to the left withou a
atrcat? strcat somehow doesn't seem the right solution for this, does
it? */
  strcat(newData, locQueue);
  /* copying new string into allocated memory */
  memcpy(locQueue, newData, strlen(locQueue));
  return 0;

}

and i get all weird terminal output, look at this:
reg@reg-laptop:~$ gcc -o fifo fifo.c
reg@reg-laptop:~$ ./fifo
strlen(locQueue):9
*** glibc detected *** ./fifo: realloc(): invalid pointer: 0xb7faff50
***
======= Backtrace: =========
/lib/tls/i686/cmov/libc.so.6[0xb7e9d3f4]
/lib/tls/i686/cmov/libc.so.6(realloc+0x242)[0xb7ea1ec2]
/lib/tls/i686/cmov/libc.so.6(realloc+0x42)[0xb7ea1cc2]
./fifo[0x80485da]
./fifo[0x80484f6]
/lib/tls/i686/cmov/libc.so.6(__libc_start_main+0xe5)[0xb7e44685]
./fifo[0x8048431]
======= Memory map: ========
08048000-08049000 r-xp 00000000 08:03 408296     /home/reg/fifo
08049000-0804a000 r--p 00000000 08:03 408296     /home/reg/fifo
0804a000-0804b000 rw-p 00001000 08:03 408296     /home/reg/fifo
09c42000-09c63000 rw-p 09c42000 00:00 0          [heap]
b7e2d000-b7e2e000 rw-p b7e2d000 00:00 0
b7e2e000-b7f86000 r-xp 00000000 08:01 1133723    /lib/tls/i686/cmov/
libc-2.8.90.so
b7f86000-b7f88000 r--p 00158000 08:01 1133723    /lib/tls/i686/cmov/
libc-2.8.90.so
b7f88000-b7f89000 rw-p 0015a000 08:01 1133723    /lib/tls/i686/cmov/
libc-2.8.90.so
b7f89000-b7f8c000 rw-p b7f89000 00:00 0
b7f90000-b7f9d000 r-xp 00000000 08:01 1116350    /lib/libgcc_s.so.1
b7f9d000-b7f9e000 r--p 0000c000 08:01 1116350    /lib/libgcc_s.so.1
b7f9e000-b7f9f000 rw-p 0000d000 08:01 1116350    /lib/libgcc_s.so.1
b7f9f000-b7fa2000 rw-p b7f9f000 00:00 0
b7fa2000-b7fbc000 r-xp 00000000 08:01 1116307    /lib/ld-2.8.90.so
b7fbc000-b7fbd000 r-xp b7fbc000 00:00 0          [vdso]
b7fbd000-b7fbe000 r--p 0001a000 08:01 1116307    /lib/ld-2.8.90.so
b7fbe000-b7fbf000 rw-p 0001b000 08:01 1116307    /lib/ld-2.8.90.so
bfeaa000-bfebf000 rw-p bffeb000 00:00 0          [stack]
Aborted
reg@reg-laptop:~$

I'm not sure why it dumps the memory map tho...

Oh hey guys,

I was jut told that the memory shall NOT be reallocated and hence stay
at the same position - So I guess I'd back at my first posting....
sorry for misleading this...

 

[Keith Thompson]

Hi Keith,

Thanks for your help!

[243 lines deleted]

Oh hey guys,

I was jut told that the memory shall NOT be reallocated and hence stay
at the same position - So I guess I'd back at my first posting....
sorry for misleading this...

When you post a followup, please trim any unnecessary quoted material;
leave just enough so that your followup makes sense to someone who
didn't see the previous article. Thanks.

 

[Barry Schwarz]

snip

I tried to correct most of the things but still couldn't get it
going...
I still would like to have an "endless" queue pointer and apparently

As already suggested, malloc and realloc are the usual tools for this.

get rid of the segmentation fault that I'm getting compiling it with
gcc 4.3.2.
My whole code including main looks like:

#include <stdio.h>
#include <pthread.h>[/QUOTE]

If this header is needed you are no longer within the confines of
standard C.  Fortunately, you don't use anything from this header.
[QUOTE]
#include <string.h>
//bool AppendToFIFO( t_FIFO* FIFO, void* newData, int newDataLength);
// global declarations
char MyQueue[1024];
int AppendToFIFO(void* newData, int newDataLength);

int main (int argc, char* argv[])
{
//setting the queue's content to 0
memset(&MyQueue,'\0', 1024);[/QUOTE]

Since MyQueue is an array, the & is merely superfluous.  However, if
you change MyQueue to a pointer as you say you want to, it becomes a
fatal error.
[QUOTE]
printf("memset MyQueue:0x%s\n",MyQueue);[/QUOTE]

The %s will cause the string to be printed up to but not including the
terminating '\0'.  Since the '\0' is the first character in the
string, nothing will be printed following the 0x.

The presence of the 0x implies you wanted to print the address of the
array.  If so, use %p and cast the address of the array to a void*. If
on the other hand you wanted to print the first byte of the array as a
hex value, then change the format to %x and the corresponding argument
to either *MyQueue or the equivalent MyQueue[0].
[QUOTE]
char* str1="TheseAre16chars";[/QUOTE]

str1 points to a string literal which is not modifiable.

The 16th character is the terminating '\0'.
[QUOTE]
//appending data loaded in number to the queue
if(AppendToFIFO(str1, 16)==0)
{
printf("1st Conent of Queue:%s\n",MyQueue);
}
else
printf("Problem appending data to Queue\n");

char* str2="hello";[/QUOTE]

Since I don't have a C99 compiler, I cannot compile your code to see
what else I missed.  Limiting the number of people who can help you is
not the best approach for a newsgroup like this.
[QUOTE]
//appending data loaded in number to the queue
if(AppendToFIFO(str2, 5)==0)[/QUOTE]

Here the terminating '\0' is not included in the count.
[QUOTE]
{
printf("2nd Conent of Queue:%s\n",MyQueue);
}
else
printf("Problem appending data to Queue\n");

return 0;
}

int AppendToFIFO(void* newData, int newDataLength)
{
/
***********************************************************************************
function: AppendToFIFO
appending newDataLength bytes from newData to MyQueue.
return value: 0 on success
1 on failure
attributes: void* newData - pointer to data to be added to queue
int newDataLength - number of bytes from newData to be
added to MyQueue
***********************************************************************************/
char* tmp;
/* Verifying data length is set > 0*/
if (newDataLength<=0)
{
printf("Error: data length is 0\n");
return 1;
}
/*checking if newData is unqual 0*/
if (strlen(newData)==0)
{
printf("no content to add to Fifo-Queue\n: %d bytes",strlen
(newData));
return 1;
}
/* calculating queue new length*/
int calclen=newDataLength+strlen(MyQueue);
printf("newDataLength:%d\nstrlen(MyQueue):%d\ncalclen:%d
\n",newDataLength,strlen(MyQueue),calclen);[/QUOTE]

I guess the undefined behavior due to the mismatch between %d and
strlen(MyQueue) that was previously discussed doesn't bother you.  It
should.
[QUOTE]
/*appending new string to current queue*/
strncat(newData, (const char*)MyQueue, strlen(MyQueue));/*PROBLEM is
here!*/[/QUOTE]

In both calls to this function, newData points to a string literal.
You attempt to modify the literal.  A segmentation fault is one of the
best possible outcomes from this undefined behavior.  

By the way, using strlen() as the third argument to strncat guarantees
that the character in the source string immediately preceding the '\0'
will NOT be copied.  That would be the 's' in the first call and the
'o' in the second.
[QUOTE]
printf("newData:%s\n",(char *)newData);
if (strlen(MyQueue)>0)
tmp=MyQueue;[/QUOTE]

I still don't know what this does.
[QUOTE]
memcpy(MyQueue, newData, calclen);[/QUOTE]

Still the wrong length.
[QUOTE]
return 0;
}[/QUOTE]

By the way, you are building a LIFO, not a FIFO.  This is because you
logic attempts to put the new data in front of the old.

For a FIFO, the entire function can be replace with
     strcat (MyQueue, newData);

 

[Ben Bacarisse]

<snip>

I tried to draw your attention to something very important. Let me
focus again on that issue since you won't get very far until you fix
it. I couls say lots of other thingg about your code but I don't want
to distract you.

/*****
SOME HEAD
fifo.c
****/
#include <stdlib.h>
#include <stdio.h>
#include <string.h>

int AppendToFIFO(char *locQueue, void* newData, int newDataLength);

int main (int argc, char* argv[])
{
char *MyQueue;

char* str1="TheseAre16chars";
/*appending data to the queue*/
if(AppendToFIFO(MyQueue, str1, 16)==0)

OK, note that the second argument in the call is str1 and that str1
points to a string literal (technically, to the array that the string
literal gives rise to, but lets keep the terminology simple for the
moment).

{
printf("1st Conent of Queue:%s\n",MyQueue);
}
else
printf("Problem appending data to Queue\n");

char* str2="hello";
/*appending data loaded in number to the queue*/
if(AppendToFIFO(MyQueue, str2, 5)==0)
{
printf("2nd Conent of Queue:%s\n",MyQueue);
}
else
printf("Problem appending data to Queue\n");

return 0;
}

int AppendToFIFO(char* locQueue, void* newData, int newDataLength)

I.e. newData points (in that first test call at least) to a string
literal.

{
/
***********************************************************************************
function: AppendToFIFO
appending newDataLength bytes from newData to MyQueue.
return value: 0 on success
!0 on failure
attributes: void* newData - pointer to data to be added to queue
int newDataLength - number of bytes from newData to be
added to MyQueue
return values:1 newDataLength is equal or smaller 0
2 length is newData is 0
***********************************************************************************/
char* tmp;
/* Verifying data length is set > 0*/
if (newDataLength<=0)
{
return 1;
}
/*checking if newData is unqual 0*/
if (strlen(newData)==0)
{
return 2;
}
printf("strlen(locQueue):%zu\n",strlen(locQueue));
/*allocating new memory size */
locQueue = realloc(locQueue, (strlen(locQueue) + newDataLength));
tmp=locQueue;
/* How can i add the new content aligned to the left withou a
atrcat? strcat somehow doesn't seem the right solution for this, does
it? */
strcat(newData, locQueue);

This won't work. Ever. If it ever /appears/ to work, it is just a
devious C compiler author trying to trip you up. newData points to a
string literal. In C, the array that is generated by writing
"something" is not modifiable. Even if it were, there is no guarantee
that there is room to add any more characters after what is there
already. You can't put anything at the end of newData.

Maybe the error is a simple one and you just don't know which way
round the arguments to strcat go, but if that is the problem I don't
know what the following memcpy is for.

 

[Ben Bacarisse]

I'm trying to write some kind of fifo algorithm.
I have a global pointer "char MyQueue[1024];" and try to append a new

You do not have pointer.  You have an array.
Yup, I actually wanted it to be a pointer so that I can copy
"endless" data into it.  But I just didn't get it going... - Can
you help me if i declared it char* MyQueue?

If you wanted to go that route, one way is to use a buffer that is
allocated and grown as needed (look up malloc and realloc in your C
reference of choice).  It is not an efficient way to do it but it
might be quite fast enough.

I tried to correct most of the things but still couldn't get it
going...

You've missed a vital comment.  Rather than comment on all the things
I could comment on I'll cut to the chase...

strncat(newData, (const char*)MyQueue, strlen(MyQueue));

You can't do this.  The string pointed to by newData is not modifiable
an even if it were there is no reason to suppose that there is room
for the content of MyQueue after it.

You did not comment on this. I have made the same point again about
some later code, so best not to comment on it here, but I think you
need to take note of it since it is an important point.

But in a queue new content should be added left aligned because it's a
fifo( first in, first out) system - no?
am i understanding it wrongly?

I can't tell for sure because you don't show the code that takes
things out of the queue, but I would write a queue of strings by
adding things to the end and removing them from the front. I.e. if I
add "abc", then "xyz" to an empty queue I'd expect to have "abcxyz".
If I then remove two characters, I'd expect to get "ab" leaving "cxyz"
in the queue.

 

[Kenny McCormack]

When you post a followup, please trim any unnecessary quoted material;
leave just enough so that your followup makes sense to someone who
didn't see the previous article. Thanks.

When you post a followup, please include something of value, not just
net-nannying. Thanks.

 

[CBFalconer]

.... snip ...

I can't tell for sure because you don't show the code that takes
things out of the queue, but I would write a queue of strings by
adding things to the end and removing them from the front. I.e.
if I add "abc", then "xyz" to an empty queue I'd expect to have
"abcxyz". If I then remove two characters, I'd expect to get "ab"
leaving "cxyz" in the queue.

The normal way of organizing a queue is as an array of <whatever>,
with a 'put' index and a 'take' index. There may also be a count
of items present, and/or a queue size value. With this you can
build queues, stacks, etc. of <whatever> quite safely. In general,
you need routines to tell the caller:

boolean isempty(queue)
boolean isfull(queue)
whatever getfrom(queue)
void putto(queue, item)
queue *initqueue(size)

The queue code itself can implement any protection needed for the
various indices etc.

 

[Ben Bacarisse]

The normal way of organizing a queue is as an array of <whatever>,
with a 'put' index and a 'take' index. There may also be a count
of items present, and/or a queue size value. With this you can
build queues, stacks, etc. of <whatever> quite safely. In general,
you need routines to tell the caller:

boolean isempty(queue)
boolean isfull(queue)
whatever getfrom(queue)
void putto(queue, item)
queue *initqueue(size)

I think you have either one * too many or at least two *s too few (or
you have a very odd interface design).

The queue code itself can implement any protection needed for the
various indices etc.

At the moment, I think the OP's difficulties lie mainly in this part
of the problem.