Dynamic C String Problem

[coosa]

Dear all,

Using the conio implementation i wanted to create a dynamic string,
whereby its size would be determined after each keyboard hit; in other
words, i don't want to ask the user to specify the the size, but
rather keep him/her typing and after each keyboard hit, the function
getch() determines whether he/she entered the ENTER key to end the
process; otherwise, increases the dynamic size or also decreases it if
the back key was hit.

For now, here is my code and I'm having
an issue; namely, the string adds extra characters not inputed and by
long strings, it doesn't hold all characters. Here is my code and
thank you in advance:

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

#define BACK_KEY 8
#define ENTER_KEY 13

typedef char * chptr;

int main (void){
int index = 0, x, y;
char input;
chptr buffer = (chptr) calloc (1 , sizeof(char));
printf("String Input => ");
do{
input = getch();
if (input != ENTER_KEY && input != BACK_KEY){
buffer[index] = input;
buffer = (chptr) realloc (buffer, ++index);
printf("%c", input);
}
else
if (input == ENTER_KEY)
buffer = (chptr) realloc (buffer,--index);
else
if (index > 0){
buffer[index-1] = '\0';
buffer = (chptr) realloc (buffer, --index);
x = wherex() - 1; y = wherey();
gotoxy(x,y); clreol();
}
}while(input != ENTER_KEY);
printf("\nString Output: %s\n", buffer);
if (buffer != NULL) free(buffer);
getch();
return EXIT_SUCCESS;
}

 

[pete]

i wanted to create a dynamic string,

in other words, i don't want to ask the user to specify the the size,

This is how I do that:

/* BEGIN line_to_string.c */

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

struct list_node {
struct list_node *next;
void *data;
};

int line_to_string(FILE *fp, char **line, size_t *size);
void list_free(struct list_node *node, void (*free_data)(void *));
void list_fputs(FILE *stream, struct list_node *node);
struct list_node *string_node(struct list_node **head,
struct list_node *tail,
char *data);

int main(void)
{
struct list_node *head, *tail;
int rc;
char *buff_ptr;
size_t buff_size;
long unsigned line_count;

puts(
"\nThis program makes and prints a list of all the lines\n"
"of text entered from standard input.\n"
"Just hit the Enter key to end,\n"
"or enter any line of characters to continue."
);
tail = head = NULL;
line_count = 0;
buff_size = 0;
buff_ptr = NULL;
while ((rc = line_to_string(stdin, &buff_ptr, &buff_size)) > 1) {
++line_count;
tail = string_node(&head, tail, buff_ptr);
if (tail == NULL) {
break;
}
puts(
"\nJust hit the Enter key to end,\n"
"or enter any other line of characters to continue."
);
}
switch (rc) {
case EOF:
if (buff_ptr != NULL && strlen(buff_ptr) > 0) {
puts("rc equals EOF\nThe string in buff_ptr is:");
puts(buff_ptr);
++line_count;
tail = string_node(&head, tail, buff_ptr);
}
break;
case 0:
puts("realloc returned a null pointer value");
if (buff_size > 1) {
puts("rc equals 0\nThe string in buff_ptr is:");
puts(buff_ptr);
++line_count;
tail = string_node(&head, tail, buff_ptr);
}
break;
default:
break;
}
if (line_count != 0 && tail == NULL) {
puts("Node allocation failed.");
puts("The last line entered didn't make it onto the list:");
puts(buff_ptr);
}
free(buff_ptr);
puts("\nThe line buffer has been freed.\n");
printf("%lu lines of text were entered.\n", line_count);
puts("They are:\n");
list_fputs(stdout, head);
list_free(head, free);
puts("\nThe list has been freed.\n");
return 0;
}

int line_to_string(FILE *fp, char **line, size_t *size)
{
int rc;
void *p;
size_t count;

count = 0;
while ((rc = getc(fp)) != EOF) {
++count;
if (count + 2 > *size) {
p = realloc(*line, count + 2);
if (p == NULL) {
if (*size > count) {
(*line)[count] = '\0';
(*line)[count - 1] = (char)rc;
} else {
ungetc(rc, fp);
}
count = 0;
break;
}
*line = p;
*size = count + 2;
}
if (rc == '\n') {
(*line)[count - 1] = '\0';
break;
}
(*line)[count - 1] = (char)rc;
}
if (rc != EOF) {
rc = count > INT_MAX ? INT_MAX : count;
} else {
if (*size > count) {
(*line)[count] = '\0';
}
}
return rc;
}

void list_free(struct list_node *node, void (*free_data)(void *))
{
struct list_node *next_node;

while (node != NULL) {
next_node = node -> next;
free_data(node -> data);
free(node);
node = next_node;
}
}

void list_fputs(FILE *stream, struct list_node *node)
{
while (node != NULL) {
fputs(node -> data, stream);
putc('\n', stream);
node = node -> next;
}
}

struct list_node *string_node(struct list_node **head,
struct list_node *tail,
char *data)
{
struct list_node *node;

node = malloc(sizeof *node);
if (node != NULL) {
node -> next = NULL;
node -> data = malloc(strlen(data) + 1);
if (node -> data != NULL) {
if (*head == NULL) {
*head = node;
} else {
tail -> next = node;
}
strcpy(node -> data, data);
} else {
free(node);
node = NULL;
}
}
return node;
}

/* END line_to_string.c */

 

[T.M. Sommers]

Dear all,

Using the conio implementation i wanted to create a dynamic string,
whereby its size would be determined after each keyboard hit; in other
words, i don't want to ask the user to specify the the size, but
rather keep him/her typing and after each keyboard hit, the function
getch() determines whether he/she entered the ENTER key to end the
process; otherwise, increases the dynamic size or also decreases it if
the back key was hit.

For now, here is my code and I'm having
an issue; namely, the string adds extra characters not inputed and by
long strings, it doesn't hold all characters. Here is my code and
thank you in advance:

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

This is a non-standard header, and should not be used in this
newsgroup.

#define BACK_KEY 8
#define ENTER_KEY 13

typedef char * chptr;

There is really no need for such a simple typedef. Much better
to just use 'char *' where needed.

int main (void){
int index = 0, x, y;
char input;
chptr buffer = (chptr) calloc (1 , sizeof(char));

You should not cast the value returned from malloc and family.
Also, sizeof char is by definition 1.

The approach you are taking of allocating memory a byte at a time
is wasteful. You should start out with a buffer big enough to
hold the expected amount of data. If you need more space, then
realloc another big block, not just an extra byte. There is also
no point in shrinking your buffer character by character.

printf("String Input => ");

This will not necessarily appear; you should fflush() stdout to
make sure it does.

do{
input = getch();
if (input != ENTER_KEY && input != BACK_KEY){
buffer[index] = input;
buffer = (chptr) realloc (buffer, ++index);

index started out as 0, and you increment it here. Your new
block of memory now has a size of 1, just like it did before.

Also, if realloc() fails, you leak the previous memory pointed to
by buffer. You need to store the return value of realloc() in a
temporary pointer variable, test that for nullity, and if
realloc() succeeds, free the old buffer.

printf("%c", input);
}
else
if (input == ENTER_KEY)
buffer = (chptr) realloc (buffer,--index);
else
if (index > 0){
buffer[index-1] = '\0';
buffer = (chptr) realloc (buffer, --index);
x = wherex() - 1; y = wherey();
gotoxy(x,y); clreol();
}
}while(input != ENTER_KEY);
printf("\nString Output: %s\n", buffer);

You never terminated your string with a '\0'.

 

[coosa]

Dear all,

Using the conio implementation i wanted to create a dynamic string,
whereby its size would be determined after each keyboard hit; in other
words, i don't want to ask the user to specify the the size, but
rather keep him/her typing and after each keyboard hit, the function
getch() determines whether he/she entered the ENTER key to end the
process; otherwise, increases the dynamic size or also decreases it if
the back key was hit.

For now, here is my code and I'm having
an issue; namely, the string adds extra characters not inputed and by
long strings, it doesn't hold all characters. Here is my code and
thank you in advance:

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

This is a non-standard header, and should not be used in this
newsgroup.

#define BACK_KEY 8
#define ENTER_KEY 13

typedef char * chptr;

There is really no need for such a simple typedef. Much better
to just use 'char *' where needed.

int main (void){
int index = 0, x, y;
char input;
chptr buffer = (chptr) calloc (1 , sizeof(char));

You should not cast the value returned from malloc and family.
Also, sizeof char is by definition 1.

The approach you are taking of allocating memory a byte at a time
is wasteful. You should start out with a buffer big enough to
hold the expected amount of data. If you need more space, then
realloc another big block, not just an extra byte. There is also
no point in shrinking your buffer character by character.

printf("String Input => ");

This will not necessarily appear; you should fflush() stdout to
make sure it does.

do{
input = getch();
if (input != ENTER_KEY && input != BACK_KEY){
buffer[index] = input;
buffer = (chptr) realloc (buffer, ++index);

index started out as 0, and you increment it here. Your new
block of memory now has a size of 1, just like it did before.

Is it? what about ++index? it's a prefix operator, so it should
increment before the allocation

Also, if realloc() fails, you leak the previous memory pointed to
by buffer. You need to store the return value of realloc() in a
temporary pointer variable, test that for nullity, and if
realloc() succeeds, free the old buffer.

printf("%c", input);
}
else
if (input == ENTER_KEY)
buffer = (chptr) realloc (buffer,--index);
else
if (index > 0){
buffer[index-1] = '\0';
buffer = (chptr) realloc (buffer, --index);
x = wherex() - 1; y = wherey();
gotoxy(x,y); clreol();
}
}while(input != ENTER_KEY);
printf("\nString Output: %s\n", buffer);

You never terminated your string with a '\0'.

if (buffer != NULL) free(buffer);
getch();
return EXIT_SUCCESS;
}

 

[T.M. Sommers]

Is it? what about ++index? it's a prefix operator, so it should
increment before the allocation

Right. Think about what happens the first time through the loop.

 

[CBFalconer]

Using the conio implementation i wanted to create a dynamic string,
whereby its size would be determined after each keyboard hit; in
other words, i don't want to ask the user to specify the the size,
but rather keep him/her typing and after each keyboard hit, the
function getch() determines whether he/she entered the ENTER key to
end the process; otherwise, increases the dynamic size or also
decreases it if the back key was hit.

For now, here is my code and I'm having an issue; namely, the
string adds extra characters not inputed and by long strings, it
doesn't hold all characters. Here is my code and thank you in
advance:

conio and getch are non-standard, and not needed for your
purposes. I have published a standard C routine ggets() which will
do what you want, and is totally portable. See:

<http://cbfalconer.home.att.net/download/>

 

[Andrew Poelstra]

Dear all,

Using the conio implementation i wanted to create a dynamic string,
whereby its size would be determined after each keyboard hit; in other
words, i don't want to ask the user to specify the the size, but
rather keep him/her typing and after each keyboard hit, the function
getch() determines whether he/she entered the ENTER key to end the
process; otherwise, increases the dynamic size or also decreases it if
the back key was hit.

How about you /don't/ use conio.h, and use getchar() for keyboard
input instead, at least when posting on this group? You will get
much more help if you keep your code within the limits of Standard
C.

For now, here is my code and I'm having
an issue; namely, the string adds extra characters not inputed and by
long strings, it doesn't hold all characters. Here is my code and
thank you in advance:

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

See my above comment, but also, what is "conio2.h"? I've never heard
of that one.

#define BACK_KEY 8
#define ENTER_KEY 13

Better: use '\b' and '\n'. See how nice the C Standard has been in
providing these things?

typedef char * chptr;

Don't do that. It's just irritating and confusing. Use char *
in your code.

int main (void){
int index = 0, x, y;
char input;
chptr buffer = (chptr) calloc (1 , sizeof(char));

1. Why calloc?
2. Don't cast the result of *alloc(). It's dangerous.
3. sizeof(type) is almost always not what you want. Use sizeof object
(which in this case would be, sizeof *buffer).
4. sizeof(char) is always 1, so in this case you can ignore #3.
5. Don't start with an inital buffer of 1. CB Falconer uses 112 in
his ggets() function, which I recommend you look at (and perhaps use in
place of your code; it is public domain).

So,
char *buffer = malloc(112);

printf("String Input => ");
do{
input = getch();

From here on I'm assuming that getch() inputs characters without

outputting them to the screen, since that's what you seem to want.
I have no idea if that's what it actually does, though.

if (input != ENTER_KEY && input != BACK_KEY){
buffer[index] = input;
buffer = (chptr) realloc (buffer, ++index);

Don't reallocate 1 more byte. Allocate 2 times as much as you had,
or maybe 4/3 as much. Also, see my above comments on *alloc() usage.

[snip remaining code]


Seriously, take a look at how ggets() is implemented. It is
completely Standard C (there's no reason not to be within those
limits in this case), and is done very well. It is one this page:
http://cbfalconer.home.att.net/download/

 

[coosa]

How about you /don't/ use conio.h, and use getchar() for keyboard
input instead, at least when posting on this group? You will get
much more help if you keep your code within the limits of Standard
C.

Of Course i would prefer a standard portable code instead of conio,
conio2 is a re-implementation and is provided as a devpack using Dev
C++ for Windows.

See my above comment, but also, what is "conio2.h"? I've never heard
of that one.


Better: use '\b' and '\n'. See how nice the C Standard has been in
providing these things?


Don't do that. It's just irritating and confusing. Use char *
in your code.


1. Why calloc?

I read that calloc will initialize the size to zero in contradiction to
malloc; i honeslty learned C once and through implementation didn't get
yet clearly the difference between them.

2. Don't cast the result of *alloc(). It's dangerous.

Well, through all the examples i read so far in lecture notes and books
as well in the web used a casting so don't blame me! :-D

3. sizeof(type) is almost always not what you want. Use sizeof object
(which in this case would be, sizeof *buffer).
4. sizeof(char) is always 1, so in this case you can ignore #3.
5. Don't start with an inital buffer of 1. CB Falconer uses 112 in
his ggets() function, which I recommend you look at (and perhaps use in
place of your code; it is public domain).

The purpose of my code is actually to produce a dynamic approach of
allocating EXACTLY what a data type requires, not more and not less. It
might sound silly to only allocate an initial size of 1 Byte, but
considering a country code that consists of only two character such as
'CA' for Canada, 'MY' for Malaysia and so third and so fourth, wouldn't
it be allocating 128 Bytes a waste to allocate such unnecessary memory
for such a variable that needs of 2 Bytes?

So,
char *buffer = malloc(112);

printf("String Input => ");
do{
input = getch();

From here on I'm assuming that getch() inputs characters without

outputting them to the screen, since that's what you seem to want.
I have no idea if that's what it actually does, though.

if (input != ENTER_KEY && input != BACK_KEY){
buffer[index] = input;
buffer = (chptr) realloc (buffer, ++index);

Don't reallocate 1 more byte. Allocate 2 times as much as you had,
or maybe 4/3 as much. Also, see my above comments on *alloc() usage.

[snip remaining code]


Seriously, take a look at how ggets() is implemented. It is
completely Standard C (there's no reason not to be within those
limits in this case), and is done very well. It is one this page:
http://cbfalconer.home.att.net/download/

 

[coosa]

How about you /don't/ use conio.h, and use getchar() for keyboard
input instead, at least when posting on this group? You will get
much more help if you keep your code within the limits of Standard
C.

Of Course, I would prefer a standard portable code instead of conio,
conio2 is a re-implementation and is provided as a devpack using Dev
C++ for Windows.

See my above comment, but also, what is "conio2.h"? I've never heard
of that one.


Better: use '\b' and '\n'. See how nice the C Standard has been in
providing these things?


Don't do that. It's just irritating and confusing. Use char *
in your code.


1. Why calloc?

I read that calloc will initialize the size to zero in contradiction to
malloc; i honeslty learned C once and through implementation didn't get
yet clearly the difference between them.

2. Don't cast the result of *alloc(). It's dangerous.

Well, through all the examples i read so far in lecture notes and books
as well in the web used a casting so don't blame me! :-D

3. sizeof(type) is almost always not what you want. Use sizeof object
(which in this case would be, sizeof *buffer).
4. sizeof(char) is always 1, so in this case you can ignore #3.
5. Don't start with an inital buffer of 1. CB Falconer uses 112 in
his ggets() function, which I recommend you look at (and perhaps use in
place of your code; it is public domain).

The purpose of my code is actually to produce a dynamic approach of
allocating EXACTLY what a data type requires, not more and not less. It
might sound silly to only allocate an initial size of 1 Byte, but
considering a country code that consists of only two characters only
such as 'CA' for Canada, 'MY' for Malaysia and so third and so fourth,
wouldn't be allocating 128 Bytes a waste to allocate such unnecessary
memory for such a variable that needs of 2 Bytes?

So,
char *buffer = malloc(112);

printf("String Input => ");
do{
input = getch();

From here on I'm assuming that getch() inputs characters without

outputting them to the screen, since that's what you seem to want.
I have no idea if that's what it actually does, though.

if (input != ENTER_KEY && input != BACK_KEY){
buffer[index] = input;
buffer = (chptr) realloc (buffer, ++index);

Don't reallocate 1 more byte. Allocate 2 times as much as you had,
or maybe 4/3 as much. Also, see my above comments on *alloc() usage.

[snip remaining code]


Seriously, take a look at how ggets() is implemented. It is
completely Standard C (there's no reason not to be within those
limits in this case), and is done very well. It is one this page:
http://cbfalconer.home.att.net/download/

 

[Richard Heathfield]

coosa said:

I read that calloc will initialize the size to zero in contradiction to
malloc;

Close. It'll initialise to all-bits-zero, which needn't mean "a value of
zero" for non-integer types (although it often does end up that way).

Well, through all the examples i read so far in lecture notes and books
as well in the web used a casting so don't blame me! :-D

You can't be blamed for being misinformed, but the cast - whilst not
actually incorrect - is unnecessary and can hide a serious bug. See
http://www.cpax.org.uk/prg/writings/casting.php for a fuller explanation.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.

 

[Flash Gordon]

Of Course, I would prefer a standard portable code instead of conio,
conio2 is a re-implementation and is provided as a devpack using Dev
C++ for Windows.

Then use portable standard functions. Andrew was even kind enough to
point you in the right direction.

I read that calloc will initialize the size to zero in contradiction to
malloc; i honeslty learned C once and through implementation didn't get
yet clearly the difference between them.

Ah, but to you really need the entire buffer cleared?

Well, through all the examples i read so far in lecture notes and books
as well in the web used a casting so don't blame me! :-D

Now you know better. Countless people drive dangerously, are you going
to drive dangerously just because many others do?

The purpose of my code is actually to produce a dynamic approach of
allocating EXACTLY what a data type requires, not more and not less. It
might sound silly to only allocate an initial size of 1 Byte, but
considering a country code that consists of only two characters only
such as 'CA' for Canada, 'MY' for Malaysia and so third and so fourth,
wouldn't be allocating 128 Bytes a waste to allocate such unnecessary
memory for such a variable that needs of 2 Bytes?

If you know there really is a small finite limit to the data it is
better to use a small finite fixed size buffer and input routines that
take the size of the buffer and, if the user enters too long a string,
do not overflow but instead flag back to the calling routine.

*alloc functions have a significant overhead and growing buffers one
byte at a time is likely to exacerbate the problems. Think about all the
housekeeping and also look up "memory fragmentation".

<snip>

Also, please snip text you are not replying to as I have done here, it
keeps posts down to a reasonable size. Of course, leave in *enough*
context for your post to stand on its own.

 

[coosa]

Then use portable standard functions. Andrew was even kind enough to
point you in the right direction.



Ah, but to you really need the entire buffer cleared?


Now you know better. Countless people drive dangerously, are you going
to drive dangerously just because many others do?


If you know there really is a small finite limit to the data it is
better to use a small finite fixed size buffer and input routines that
take the size of the buffer and, if the user enters too long a string,
do not overflow but instead flag back to the calling routine.

*alloc functions have a significant overhead and growing buffers one
byte at a time is likely to exacerbate the problems. Think about all the
housekeeping and also look up "memory fragmentation".

<snip>

Well some thing still bothers me; in C++, "Shrink to Fit" well known
for the STL's Sequence Containers including the Standard C++ String
Class. I would go along with your method and do it the way you
recommended, but I'll end up shrinking extra space to fit the actual
length; either way, my old way byte by byte or this way, the result is
the same.

 

[apoelstra]

I read that calloc will initialize the size to zero in contradiction to
malloc; i honeslty learned C once and through implementation didn't get
yet clearly the difference between them.


Well, through all the examples i read so far in lecture notes and books
as well in the web used a casting so don't blame me! :-D

Well, you came to the right place, then. This group will tell you
all sorts of things that the world teaches that are wrong. You
should tell your teacher not to cast malloc() if your lecture notes
really say to do so.

The purpose of my code is actually to produce a dynamic approach of
allocating EXACTLY what a data type requires, not more and not less. It
might sound silly to only allocate an initial size of 1 Byte, but
considering a country code that consists of only two character such as
'CA' for Canada, 'MY' for Malaysia and so third and so fourth, wouldn't
it be allocating 128 Bytes a waste to allocate such unnecessary memory
for such a variable that needs of 2 Bytes?

Not necessarily. The internal implementation of malloc() may very well
always allocate 16 bytes, or some other large number, simply to justify
the bookkeeping required to allocate memory. So, you're wasting space
by /not/ guaranteeing yourself access to all that memory.

 

[goose]

Andrew Poelstra wrote:

I read that calloc will initialize the size to zero in contradiction to

You perhaps mean "initialise the buffer to all zeros"?

Well, through all the examples i read so far in lecture notes and books
as well in the web used a casting so don't blame me! :-D

Well, you've had a stroke of luck running into this newsgroup
then

All those examples in lecture notes and books as well as in(sic)
the web were either
a) Wrong
OR
b) C++ examples, not C examples.

goose,