Read dynamic string

[Sillaba atona]

I use this code to read dynamic string:

char *s1;
.......
puts("Inserire una stringa: ");
while((*s1++=getchar())!='\n');
*s1='\0';

The compilation (ANSI C) is OK but I receive an error during the execution.

The problem is not present if I use a static array.

I cannot find the error.

[Malcolm]

"Sillaba atona" <NO****@tin.it> wrote in message

char *s1;
......
puts("Inserire una stringa: ");
while((*s1++=getchar())!='\n');
*s1='\0';

The compilation (ANSI C) is OK but I receive an error during the
execution.

The problem is not present if I use a static array.

I cannot find the error.

s1 points to nowhere in particular. You then start overwriting this random
memory location with characters. Usually the result will be a crash
(probably a message saying "segmentation fault").
If you say s1 = malloc(1000) then s1 will point to 1000 chars reserved for
you, and the program won't crash until you exceed that figure. (You can keep
a count and call realloc(), if you want to be able to read an arbitrary
string as long as the computer's meory allows).

[Walter Roberson]

In article <43***********************@reader1.news.tin.it>,
Sillaba atona <NO****@tin.it> wrote:

I use this code to read dynamic string: char *s1;
......
puts("Inserire una stringa: ");
while((*s1++=getchar())!='\n');
You have not allocated any storage. s1 is an uninitialized pointer;
you have to point it to a block of memory before you can use that code.

Except that that code doesn't take into account the possibility of
a really long string, and if you pre-allocate the memory then no matter
how much you allocate, the user might enter something larger.
Therefore you either need to limit the length that you will pay
attention to, or else you need to use a scheme in which the allocated
memory is grown as needed.
*s1='\0'; The compilation (ANSI C) is OK but I receive an error during the execution.
A good compiler would warn that s1 was potentially uninitialized, but
such a warning is not -required- by the standard.
The problem is not present if I use a static array.

Change back to a static, and then have the user paste in (say)
32K of text, and see whether you still say the problem is "not present"
when you use a static array.
--
"No one has the right to destroy another person's belief by
demanding empirical evidence." -- Ann Landers

[Toni Uusitalo]

Malcolm wrote:

If you say s1 = malloc(1000) then s1 will point to 1000 chars reserved for
you, and the program won't crash until you exceed that figure. (You can keep
a count and call realloc(), if you want to be able to read an arbitrary
string as long as the computer's meory allows).

Or you can keep reading into unallocated buffer until your computer meows.

;-)

with respect,
Toni Uusitalo

[Daniel Fischer]

Hi,

(You can keep a count and call realloc(), if you want to be able to read
an arbitrary string as long as the computer's meory allows).

there's also a neat way that involves using recursion to allocate
additional memory on the stack and putting the parts together on return,
thereby avoiding heap fragmentation.
Daniel

[Christopher Benson-Manica]

Sillaba atona <NO****@tin.it> wrote:

char *s1;
......
puts("Inserire una stringa: ");
while((*s1++=getchar())!='\n');
*s1='\0'; The compilation (ANSI C) is OK but I receive an error during the execution.

It's wrong, as others have indicated.

You might find Chuck Falconer's ggets() routine, available at
cbfalconer.home.att.net/download/ggets.zip, to be helpful. Chuck is
(or at least was, he seems to have been missing for some time) a
regular contributor here, so you can use the code with confidence
(per the license, which I believe is GPL).

--
Christopher Benson-Manica | I *should* know what I'm talking about - if I
ataru(at)cyberspace.org | don't, I need to know. Flames welcome.

[Malcolm]

"Daniel Fischer" <sp**@erinye.com> wrote

(You can keep a count and call realloc(), if you want to be able to read
an arbitrary string as long as the computer's meory allows).

there's also a neat way that involves using recursion to allocate
additional memory on the stack and putting the parts together on return,
thereby avoiding heap fragmentation.

Could you post a getline() function (get a line from a file, of arbitrary
length) that uses this?

[Toni Uusitalo]

Daniel Fischer wrote:

Hi,

(You can keep a count and call realloc(), if you want to be able to read
an arbitrary string as long as the computer's meory allows).

there's also a neat way that involves using recursion to allocate
additional memory on the stack and putting the parts together on return,
thereby avoiding heap fragmentation.

This is possible but I doubt it's worth the hassle (I admit it sounds
like neat trick).

I think something that reuses existing buffer would be enough for
avoiding constant buffer fiddling/reallocating.

Something like this:

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

struct GetLine {
FILE *f;
char *buf;
size_t bufsize;
};

int GetLine_Init(struct GetLine *gl, FILE *f)
{
gl->f = f;
gl->bufsize = 256;
gl->buf = malloc(gl->bufsize);
return (gl->buf) ? 1 : 0;
}

void GetLine_Destroy(struct GetLine *gl)
{
free(gl->buf);
}

char *GetLine_Read(struct GetLine *gl)
{
size_t i=0;
int ch;

while(gl->buf) {
for(; i<gl->bufsize; gl->buf[i++]=ch) {
ch = fgetc(gl->f);
if (ch == EOF || ch == '\n') {
if (!i && ch == EOF) return NULL;
gl->buf[i] = '\0';
return gl->buf;
}
}
gl->bufsize += gl->bufsize;
gl->buf = realloc(gl->buf, gl->bufsize);
}
return NULL;
}

int main(int argc, char* argv[])
{
struct GetLine gl;
char *line;
FILE *f = fopen("test.txt", "r");

if (!f) return (EXIT_FAILURE);
if (!GetLine_Init(&gl, f)) return (EXIT_FAILURE);

while((line=GetLine_Read(&gl)))
puts(line);

GetLine_Destroy(&gl);
fclose(f);
return 0;
}

with respect,
Toni Uusitalo

[Joe Wright]

Daniel Fischer wrote:

Hi,

(You can keep a count and call realloc(), if you want to be able to read
an arbitrary string as long as the computer's meory allows).

there's also a neat way that involves using recursion to allocate
additional memory on the stack and putting the parts together on return,
thereby avoiding heap fragmentation.
Daniel

I'd love to see that. Can you post the code?

--
Joe Wright
"Everything should be made as simple as possible, but not simpler."
--- Albert Einstein ---

[Daniel Fischer]

On Sun, 27 Nov 2005 18:27:12 -0500, Joe Wright wrote:

there's also a neat way that involves using recursion to allocate
additional memory on the stack and putting the parts together on return,
thereby avoiding heap fragmentation.
Daniel

I'd love to see that. Can you post the code?

Sure:

------------------------->8---------------------------

/*
* rfgets.c
* dynamically allocating fgets
* daniel.fischer at iitb.fraunhofer.de
*/

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

char *rfg(FILE * f, int l) {
char b[32], *p;
int x;

if(!fgets(b, sizeof(b), f)) return 0;
x = strlen(b);
if(b[x - 1] != '\n' && (p = rfg(f, l + x))) {
p -= x;
memcpy(p, b, x);
return p;
}
p = (char *) malloc(l + x + 1) + l;
strcpy(p, b);
return p;
}

char *rfgets(FILE * f) {
return rfg(f, 0);
}

------------------------->8---------------------------

Usage:

char *line = rfgets(stdin);
...
free(line); // don't forget
Daniel

[Christopher Benson-Manica]

Daniel Fischer <sp**@erinye.com> wrote:

Bonus points for ingenuity. But...

char *rfg(FILE * f, int l) {
char b[32], *p;
int x; if(!fgets(b, sizeof(b), f)) return 0;
x = strlen(b);
if(b[x - 1] != '\n' && (p = rfg(f, l + x))) {
p -= x; /* NB */ memcpy(p, b, x);
return p;
}
p = (char *) malloc(l + x + 1) + l;
This cast is neither necessary nor desirable; search this group's
archives for any of several lengthy discussions as to why.
strcpy(p, b);
return p;
I believe the value that should be returned is p+l. The code is
certainly broken as is, as the line indicated generates a pointer x
bytes before the beginning of the malloc()'ed space.
}

--
Christopher Benson-Manica | I *should* know what I'm talking about - if I
ataru(at)cyberspace.org | don't, I need to know. Flames welcome.

[Barry Schwarz]

On Mon, 28 Nov 2005 13:14:55 +0100, Daniel Fischer <sp**@erinye.com>
wrote:

On Sun, 27 Nov 2005 18:27:12 -0500, Joe Wright wrote:

there's also a neat way that involves using recursion to allocate
additional memory on the stack and putting the parts together on return,
thereby avoiding heap fragmentation.
Daniel
I'd love to see that. Can you post the code?

Sure:

------------------------->8---------------------------

/*
* rfgets.c
* dynamically allocating fgets
* daniel.fischer at iitb.fraunhofer.de
*/

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

char *rfg(FILE * f, int l) {
char b[32], *p;
int x;

if(!fgets(b, sizeof(b), f)) return 0;
x = strlen(b);
if(b[x - 1] != '\n' && (p = rfg(f, l + x))) {
p -= x;
memcpy(p, b, x);
return p;
}
p = (char *) malloc(l + x + 1) + l;

I give up. Which ones are ones and which are ells?
strcpy(p, b);
return p;
}

char *rfgets(FILE * f) {
return rfg(f, 0);
}

------------------------->8---------------------------

Usage:

char *line = rfgets(stdin);
...
free(line); // don't forget
Daniel

<<Remove the del for email>>

[S.Tobias]

Christopher Benson-Manica <at***@nospam.cyberspace.org> wrote:

Daniel Fischer <sp**@erinye.com> wrote:

Bonus points for ingenuity. But...

char *rfg(FILE * f, int l) {
char b[32], *p;
int x;

if(!fgets(b, sizeof(b), f)) return 0;
x = strlen(b);
if(b[x - 1] != '\n' && (p = rfg(f, l + x))) {
p -= x;

/* NB */

memcpy(p, b, x);
return p;
}
p = (char *) malloc(l + x + 1) + l;

This cast is neither necessary nor desirable; search this group's
archives for any of several lengthy discussions as to why.

It is necessary here, you've missed `+ l' after malloc, which is
also an answer to your second query below.

strcpy(p, b);
return p;

I believe the value that should be returned is p+l. The code is
certainly broken as is, as the line indicated generates a pointer x
bytes before the beginning of the malloc()'ed space.

}

--
Stan Tobias
mailx `echo si***@FamOuS.BedBuG.pAlS.INVALID | sed s/[[:upper:]]//g`

[Christopher Benson-Manica]

S.Tobias <si***@famous.bedbug.pals.invalid> wrote:

It is necessary here, you've missed `+ l' after malloc, which is
also an answer to your second query below.

Darn. I thought I was being observant, but turns out I need glasses
after all. (I blame it all on the use of lowercase 'l'.)

--
Christopher Benson-Manica | I *should* know what I'm talking about - if I
ataru(at)cyberspace.org | don't, I need to know. Flames welcome.

[Randy Howard]

Christopher Benson-Manica wrote
(in article <dm**********@chessie.cirr.com>):

S.Tobias <si***@famous.bedbug.pals.invalid> wrote:

It is necessary here, you've missed `+ l' after malloc, which is
also an answer to your second query below.

Darn. I thought I was being observant, but turns out I need glasses
after all. (I blame it all on the use of lowercase 'l'.)

Pick a better font (or terminal). :-)
--
Randy Howard (2reply remove FOOBAR)
"The power of accurate observation is called cynicism by those
who have not got it." - George Bernard Shaw

[Jordan Abel]

On 2005-11-29, Christopher Benson-Manica <at***@nospam.cyberspace.org> wrote:

S.Tobias <si***@famous.bedbug.pals.invalid> wrote:

It is necessary here, you've missed `+ l' after malloc, which is
also an answer to your second query below.

Darn. I thought I was being observant, but turns out I need glasses
after all. (I blame it all on the use of lowercase 'l'.)

You couldn't do it if it were a 1 (one) or an I (eye), either, though.

[Christopher Benson-Manica]

Toni Uusitalo <to***************@suxpan.nubigtime> wrote:

I think something that reuses existing buffer would be enough for
avoiding constant buffer fiddling/reallocating.

It is enough, but the recursive version uses repeated recursive calls
and memcpy() where your version uses realloc(). realloc() is
generally expensive performance-wise, so I think the recursive
version's avoidance of it is probably a good thing.

--
Christopher Benson-Manica | I *should* know what I'm talking about - if I
ataru(at)cyberspace.org | don't, I need to know. Flames welcome.

[Toni Uusitalo]

Christopher Benson-Manica wrote:

Toni Uusitalo <to***************@suxpan.nubigtime> wrote:

I think something that reuses existing buffer would be enough for
avoiding constant buffer fiddling/reallocating.

It is enough, but the recursive version uses repeated recursive calls
and memcpy() where your version uses realloc(). realloc() is
generally expensive performance-wise, so I think the recursive
version's avoidance of it is probably a good thing.

Yes. Recursive version seems very clever! Recursion starts
to affect performance at some point I recall from some code I wrote,
I think that particular recursion was something like 30 levels deep on
my win platform that started to affect performance crucially.

Ideally my version uses realloc sparingly. If you need to store
line(s) returned from Readline_Read you should use strdup() etc.
and continue using already allocated buffer for reading lines.

some enchancements:

+ #define GetLine_Linelen(gl) ((gl)->lineLen)

struct GetLine {
FILE *f;
char *buf;
size_t bufsize;
+ size_t lineLen;
};
and new GetLine_Read (which simply uses gl->lineLen instead of i):
char *GetLine_Read(struct GetLine *gl)
{
int ch;
gl->lineLen=0;

while(gl->buf) {
for(; gl->lineLen<gl->bufsize; gl->buf[gl->lineLen++]=ch) {
ch = fgetc(gl->f);
if (ch == EOF || ch == '\n') {
if (!gl->lineLen && ch == EOF) return NULL;
gl->buf[gl->lineLen] = '\0';
return gl->buf;
}
}
gl->bufsize += gl->bufsize;
gl->buf = realloc(gl->buf, gl->bufsize);
}
return NULL;
}

Now it's possible to use memcpy after GetLine_Read instead of strdup()

with respect,
Toni Uusitalo

[Christopher Benson-Manica]

Jordan Abel <jm****@purdue.edu> wrote:

You couldn't do it if it were a 1 (one) or an I (eye), either, though.

Of course, but the problem is that I completely overlooked the 'l',
which would have been less likely if the variable had been given a
more descriptive name. Hopefully real code would have had a helpful
comment for the unobservant...

--
Christopher Benson-Manica | I *should* know what I'm talking about - if I
ataru(at)cyberspace.org | don't, I need to know. Flames welcome.

[those who know me have no need of my name]

in comp.lang.c i read:

i elided most of the code -- sometimes this is a mistake, though i hope not
-- only the bits with nits remain.

char *rfg(FILE * f, int l) {
int makes some sense, but i happen to prefer size_t as there is no meaning
in a negative length or offset in this code.

char *rfg(FILE * f, size_t l) {

also, this is now a very short name to have with external linkage. i would
probably change to internal linkage, or rename it rfgets_r -- heh, perhaps
both.
int x;
nothing wrong with it, but again as you cannot have a negative string
length i tend to prefer a size_t.

size_t x;
if(b[x - 1] != '\n' && (p = rfg(f, l + x))) {
what if the first byte fgets returns is a null byte? better check for zero
length just in case.

also, what if rfg returns a null pointer (malloc failed)? do you really
want to just chuck the last part of the line away and make believe it ends
at the current block?

sometimes losing as little data as possible is important, in which case
your code with a length check added would be fine:

if(0 < x && b[x - 1] != '\n' && (p = rfg(f, l + x))) {

but i think it's usually better to back propagate the malloc failure:

if(0 < x && b[x - 1] != '\n') {
if (0 == (p = rfg(f, l + x))) return 0;
p = (char *) malloc(l + x + 1) + l;
strcpy(p, b);

what if the malloc fails? (the cast just to do the arithmetic is ugly
enough to want a change, even though it is valid -- as long as malloc
doesn't fail.)

if (0 == (p = malloc(l + x + 1))) return 0;
p += l;
strcpy(p, b);

--
a signature

[Toni Uusitalo]

those who know me have no need of my name wrote:

in comp.lang.c i read:

i elided most of the code -- sometimes this is a mistake, though i hope not
-- only the bits with nits remain.

char *rfg(FILE * f, int l) {

int makes some sense, but i happen to prefer size_t as there is no meaning
in a negative length or offset in this code.

char *rfg(FILE * f, size_t l) {

also, this is now a very short name to have with external linkage. i would
probably change to internal linkage, or rename it rfgets_r -- heh, perhaps
both.

I would change its name to wof (write-only function)
(http://en.wikipedia.org/wiki/Write-only_language)
;-)

Well maybe I'm just jealous cos I couldn't come up with something as
clever as this function ;-)

with respect,
Toni Uusitalo

[Toni Uusitalo]

Toni Uusitalo wrote:

those who know me have no need of my name wrote:

in comp.lang.c i read:

i elided most of the code -- sometimes this is a mistake, though i
hope not
-- only the bits with nits remain.

char *rfg(FILE * f, int l) {

int makes some sense, but i happen to prefer size_t as there is no
meaning
in a negative length or offset in this code.

char *rfg(FILE * f, size_t l) {

also, this is now a very short name to have with external linkage. i
would
probably change to internal linkage, or rename it rfgets_r -- heh,
perhaps
both.

I would change its name to wof (write-only function)
(http://en.wikipedia.org/wiki/Write-only_language)
;-)

Oh sorry, of course it must be
WriteOnlyFunctionWhichIsCleverTrustMe()
to avoid too short name

with respect,
Toni Uusitalo

[Daniel Fischer]

those who know me have no need of my name!

what if the malloc fails? (the cast just to do the arithmetic is ugly
enough to want a change, even though it is valid -- as long as malloc
doesn't fail.)

One could argue that if malloc fails, writing to 0+l results in
fail-fast-by-segfault behaviour :)

But I guess you're right.
Daniel

[Richard Bos]

Daniel Fischer <sp**@erinye.com> wrote:

those who know me have no need of my name!

what if the malloc fails? (the cast just to do the arithmetic is ugly
enough to want a change, even though it is valid -- as long as malloc
doesn't fail.)

One could argue that if malloc fails, writing to 0+l results in
fail-fast-by-segfault behaviour :)

_May_ result in fail-fast-by-segfault behaviour. May also, e.g., result
in fail-late-and-disastrously-by-overwritten-interrupt-vector behaviour.

Richard

[Flash Gordon]

Daniel Fischer wrote:

those who know me have no need of my name!

what if the malloc fails? (the cast just to do the arithmetic is ugly
enough to want a change, even though it is valid -- as long as malloc
doesn't fail.)
One could argue that if malloc fails, writing to 0+l results in
fail-fast-by-segfault behaviour :)

One could argue that, but one would not be right on all systems.
But I guess you're right.

He is indeed.
--
Flash Gordon
Living in interesting times.
Although my email address says spam, it is real and I read it.