opinions on code

[jaso]

I've written a function that reads multiple
lines and puts it in a string.
What do you think of it, is it correct?
Thanks!

#define BUFSIZE 1024
int read_multiline(char **str)
{
char inputbuf[BUFSIZE];
char *tmp;
size_t size = BUFSIZE;

*str = malloc(size);
if (*str == NULL)
return -1;

(*str)[0] = '\0';
fgets(inputbuf, BUFSIZE, stdin);
while (inputbuf[0] != '\n') {
/* Allocate more memory if needed */
if (strlen(*str) + strlen(inputbuf) >= size) {
size += BUFSIZE;
tmp = realloc(*str, size);
if (tmp == NULL) {
free(*str);
return -1;
}
else
*str = tmp;
}
strcat(*str, inputbuf);
fgets(inputbuf, BUFSIZE, stdin);
}

return 0;
}

 

[spibou]

I've written a function that reads multiple
lines and puts it in a string.
What do you think of it, is it correct?
Thanks!

#define BUFSIZE 1024
int read_multiline(char **str)
{
char inputbuf[BUFSIZE];
char *tmp;
size_t size = BUFSIZE;

*str = malloc(size);
if (*str == NULL)
return -1;

(*str)[0] = '\0';
fgets(inputbuf, BUFSIZE, stdin);
while (inputbuf[0] != '\n') {
/* Allocate more memory if needed */
if (strlen(*str) + strlen(inputbuf) >= size) {
size += BUFSIZE;
tmp = realloc(*str, size);
if (tmp == NULL) {
free(*str);
return -1;
}
else
*str = tmp;
}
strcat(*str, inputbuf);
fgets(inputbuf, BUFSIZE, stdin);
}

return 0;
}

Why not test it first and then ask us ? But since you decided to ask
first , it would have helped if you had given more details on what the
function is supposed to do. For example I take it that the function is
supposed to return if it reads an empty line.

I have spotted 2 mistakes and there may be more:
1) You do not check the return value of fgets() therefore you won't
know
if EOF has been reached.
2) It is possible , albeit unlikely , that a line will have size
exactly BUFSIZE-1
so your programme will put the line into inputbuf[] and when the loop
gets repeated
it will read '\n' and the function will exit although an empty line has
not been read.

Apart from these I note also that your algorithm is inefficient.
There's no reason
to put the read lines first inside inputbuf[] and then copy them into
your main buffer ;
you should put them straight into the main buffer as you read them.
Trying reading
one character at a time using getchar().

 

[spibou]

I've written a function that reads multiple
lines and puts it in a string.
What do you think of it, is it correct?
Thanks!

#define BUFSIZE 1024
int read_multiline(char **str)
{
char inputbuf[BUFSIZE];
char *tmp;
size_t size = BUFSIZE;

*str = malloc(size);
if (*str == NULL)
return -1;

(*str)[0] = '\0';
fgets(inputbuf, BUFSIZE, stdin);
while (inputbuf[0] != '\n') {
/* Allocate more memory if needed */
if (strlen(*str) + strlen(inputbuf) >= size) {
size += BUFSIZE;
tmp = realloc(*str, size);
if (tmp == NULL) {
free(*str);
return -1;
}
else
*str = tmp;
}
strcat(*str, inputbuf);
fgets(inputbuf, BUFSIZE, stdin);
}

return 0;
}

Why not test it first and then ask us ? But since you decided to ask
first , it would have helped if you had given more details on what the
function is supposed to do. For example I take it that the function is
supposed to return if it reads an empty line.

I have spotted 2 mistakes and there may be more:
1) You do not check the return value of fgets() therefore you won't
know
if EOF has been reached.
2) It is possible , albeit unlikely , that a line will have size
exactly BUFSIZE-1
so your programme will put the line into inputbuf[] and when the loop
gets repeated
it will read '\n' and the function will exit although an empty line has
not been read.

Apart from these I note also that your algorithm is inefficient.
There's no reason
to put the read lines first inside inputbuf[] and then copy them into
your main buffer ;
you should put them straight into the main buffer as you read them.
Trying reading
one character at a time using getchar().

Another remark: your code seems to rely on the assumption that a
character with value 0 won't be read from stdin. This is not guaranteed
and it's easy enough to modify your code so that it doesn't rely on
that assumption.