Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Taking a stab at getline

Reply
Thread Tools

Taking a stab at getline

 
 
Barry Schwarz
Guest
Posts: n/a
 
      02-10-2013
On Sat, 09 Feb 2013 19:02:32 +0100, Rosario1903
<(E-Mail Removed)> wrote:

>On Thu, 07 Feb 2013 13:42:12 +0100, Noob <root@127.0.0.1> wrote:
>
>>Hello group,
>>
>>Here's my attempt at writing a "get_line" implementation, which
>>reads an entire line from a file stream, dynamically allocating
>>the space needed to store said line.
>>
>>Since this is for my own personal use, I didn't bother handling
>>out-of-memory conditions elegantly. I just kick the bucket.
>>("We all kick the bucket in the end, in the end.")
>>
>>EOF is signaled by returning NULL.
>>
>>I'd like to hear suggestions/criticism.

>
>// ritorna un puntatore da liberarsi tramite free()
>// ritorna 0 per errore
>char* __export GetLineI(char* testo)


What is __export? I don't se it in n1256 (C99) or n1570 (C11).

>{unsigned s, i;
> int c;
> char *p,*t;
> s=1024; p=malloc(s+4);


It would make your code much easier to read if you invested in some
horizontal white space.

You might also consider sticking to one statement per line.

> if(p==0) return 0;
> printf("%s", testo); fflush(stdout);


The second time you call this function, wouldn't like the output to be
on a new line? Does testo always terminate with a '\n'?

> for(i=0; (c=getchar())!=EOF ; )
> {if(i>=s)
> {s=s+1024;
> t=realloc(p, s+4);


What is the purpose of the extra 4 bytes?

> if(t==0||(int)s<0)


If s exceeds INT_MAX, casting it to an int need not produce a negative
value. You would do better to simply compare s > INT_MAX. On the
other hand, if you made i unsigned, you wouldn't care. You can
probably assume that realloc will fail before s > UINT_MAX but if you
really think you will lines with gigabyte lengths you could impose an
arbitrary limit.

> {free(p); return 0;}
> p=t;
> }
> p[i]=c; ++i;
> if(c=='\n')
> break;
> }
> p[i]=0;
> return p;
>}
>


--
Remove del for email
 
Reply With Quote
 
 
 
 
Rosario1903
Guest
Posts: n/a
 
      02-10-2013
On Sat, 09 Feb 2013 23:16:26 -0800, Barry Schwarz wrote:

>On Sat, 09 Feb 2013 19:02:32 +0100, Rosario1903
><(E-Mail Removed)> wrote:
>
>>On Thu, 07 Feb 2013 13:42:12 +0100, Noob <root@127.0.0.1> wrote:
>>
>>>Hello group,
>>>
>>>Here's my attempt at writing a "get_line" implementation, which
>>>reads an entire line from a file stream, dynamically allocating
>>>the space needed to store said line.
>>>
>>>Since this is for my own personal use, I didn't bother handling
>>>out-of-memory conditions elegantly. I just kick the bucket.
>>>("We all kick the bucket in the end, in the end.")
>>>
>>>EOF is signaled by returning NULL.
>>>
>>>I'd like to hear suggestions/criticism.

>>
>>// ritorna un puntatore da liberarsi tramite free()
>>// ritorna 0 per errore
>>char* __export GetLineI(char* testo)

>
>What is __export? I don't se it in n1256 (C99) or n1570 (C11).
>
>>{unsigned s, i;
>> int c;
>> char *p,*t;
>> s=1024; p=malloc(s+4);

>
>It would make your code much easier to read if you invested in some
>horizontal white space.
>
>You might also consider sticking to one statement per line.


i'm not agree, for me in a line there would be instructions,
statements that do togheter something simple, as for example swap :
"t=a; a=b; b=t;"
would be ok in one line

 
Reply With Quote
 
 
 
 
Rosario1903
Guest
Posts: n/a
 
      02-10-2013
On Sat, 09 Feb 2013 23:16:26 -0800, Barry Schwarz wrote:

>On Sat, 09 Feb 2013 19:02:32 +0100, Rosario1903
><(E-Mail Removed)> wrote:


>>
>>// ritorna un puntatore da liberarsi tramite free()
>>// ritorna 0 per errore
>>char* __export GetLineI(char* testo)

>
>What is __export? I don't se it in n1256 (C99) or n1570 (C11).
>
>>{unsigned s, i;
>> int c;
>> char *p,*t;
>> s=1024; p=malloc(s+4);

>
>It would make your code much easier to read if you invested in some
>horizontal white space.
>
>You might also consider sticking to one statement per line.
>
>> if(p==0) return 0;
>> printf("%s", testo); fflush(stdout);

>
>The second time you call this function, wouldn't like the output to be
>on a new line? Does testo always terminate with a '\n'?
>
>> for(i=0; (c=getchar())!=EOF ; )
>> {if(i>=s)
>> {s=s+1024;
>> t=realloc(p, s+4);

>
>What is the purpose of the extra 4 bytes?


i like some extra space and possibly not understand well or fear the 0
bytes case solution

>> if(t==0||(int)s<0)

>
>If s exceeds INT_MAX, casting it to an int need not produce a negative
>value. You would do better to simply compare s > INT_MAX.


yes but i not like write too much, and her in 386 cpu all 2 compilers
i use would be ok

>On the
>other hand, if you made i unsigned, you wouldn't care.


i don't like overflow

>You can
>probably assume that realloc will fail before s > UINT_MAX but if you


what about s+4 overflow s unsigned?

>really think you will lines with gigabyte lengths you could impose an
>arbitrary limit.



>> {free(p); return 0;}
>> p=t;
>> }
>> p[i]=c; ++i;
>> if(c=='\n')
>> break;
>> }
>> p[i]=0;
>> return p;
>>}
>>


all error stream check is left by exercise for the reader...

 
Reply With Quote
 
Ben Bacarisse
Guest
Posts: n/a
 
      02-10-2013
Barry Schwarz <(E-Mail Removed)> writes:

> On Sat, 09 Feb 2013 19:02:32 +0100, Rosario1903
> <(E-Mail Removed)> wrote:

<snip>
>> s=1024; p=malloc(s+4);

<snip>
>> if(p==0) return 0;
>> printf("%s", testo); fflush(stdout);

<snip>
>> for(i=0; (c=getchar())!=EOF ; )
>> {if(i>=s)
>> {s=s+1024;
>> t=realloc(p, s+4);

>
> What is the purpose of the extra 4 bytes?


That bugged me until I saw that at least +1 is needed because of the way
the rest of the code is written: if the buffer is of size 's', a newline
written to the last byte will case the null to be written one past the
end. I guessed that having discovered a problem in testing, the +4 is
some sort of insurance.

>> if(t==0||(int)s<0)

<snip>
>> {free(p); return 0;}
>> p=t;
>> }
>> p[i]=c; ++i;
>> if(c=='\n')
>> break;
>> }
>> p[i]=0;
>> return p;
>>}


--
Ben.
 
Reply With Quote
 
Rosario1903
Guest
Posts: n/a
 
      02-11-2013
On Sat, 09 Feb 2013 19:02:32 +0100, Rosario1903
<(E-Mail Removed)> wrote:

>On Thu, 07 Feb 2013 13:42:12 +0100, Noob <root@127.0.0.1> wrote:
>
>>Hello group,
>>
>>Here's my attempt at writing a "get_line" implementation, which
>>reads an entire line from a file stream, dynamically allocating
>>the space needed to store said line.
>>
>>Since this is for my own personal use, I didn't bother handling
>>out-of-memory conditions elegantly. I just kick the bucket.
>>("We all kick the bucket in the end, in the end.")
>>
>>EOF is signaled by returning NULL.
>>
>>I'd like to hear suggestions/criticism.

>
>// ritorna un puntatore da liberarsi tramite free()
>// ritorna 0 per errore
>char* __export GetLineI(char* testo)
>{unsigned s, i;
> int c;
> char *p,*t;


i add some error checking...
What about

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

// ritorna un puntatore da liberarsi tramite free()
// 0 per errore
char* GetLineI(char* testo)
{unsigned s, i, vMax=UINT_MAX-1032; // 1024+4+1 [+ 3]=1032
int c;
char *p,*t;

s=1024; p=malloc(s+4);
if(p==0) return 0;
if(printf("%s", testo)<0)
{la: free(p); return 0;}
if(fflush(stdout)!=0)
goto la;
for(i=0; (c=getchar())!=EOF ; )
{if(i>=s)
{if(s>=vMax)
goto la;
s=s+1024;
t=realloc(p, s+4);
if(t==0)
goto la;
p=t;
}
p[i]=c; ++i;
if(c=='\n')
break;
}
if(ferror(stdin))
goto la;
p[i]=0;
return p;
}

int main(void)
{char *r;
r=GetLineI("Inserisci stringa: ");
if(r==0)
printf("Errore\n");
else {printf("Hai inserito: [%s]\n", r);
free(r);
}
return 0;
}


 
Reply With Quote
 
Rosario1903
Guest
Posts: n/a
 
      02-11-2013
On Sat, 09 Feb 2013 19:02:32 +0100, Rosario1903
<(E-Mail Removed)> wrote:


>// ritorna un puntatore da liberarsi tramite free()
>// ritorna 0 per errore
>char* __export GetLineI(char* testo)
>{unsigned s, i;
> int c;
> char *p,*t;
> s=1024; p=malloc(s+4);
> if(p==0) return 0;
> printf("%s", testo); fflush(stdout);
> for(i=0; (c=getchar())!=EOF ; )
> {if(i>=s)
> {s=s+1024;
> t=realloc(p, s+4);
> if(t==0||(int)s<0)
> {free(p); return 0;}


here there is one error if t!=0 and (int)s<0
free(p) free the wrong pointer...

> p=t;
> }
> p[i]=c; ++i;
> if(c=='\n')
> break;
> }
> p[i]=0;
> return p;
>}
>


 
Reply With Quote
 
Ben Bacarisse
Guest
Posts: n/a
 
      02-11-2013
Rosario1903 <(E-Mail Removed)> writes:
<snip>
> i add some error checking...
> What about

<snip>
> if(printf("%s", testo)<0)
> {la: free(p); return 0;}


If you are going to test the return form printf it would be better to
test for all errors. Even a non-negative return < strlen(testo) is an
error.

But fputs is both easier to use (no format) and easier to check for
errors:

if (fputs(testo, stdout) == EOF) ...

<snip>
--
Ben.
 
Reply With Quote
 
Rosario1903
Guest
Posts: n/a
 
      02-11-2013
On Mon, 11 Feb 2013 13:30:11 +0000, Ben Bacarisse
<(E-Mail Removed)> wrote:

>Rosario1903 <(E-Mail Removed)> writes:
><snip>
>> i add some error checking...
>> What about

><snip>
>> if(printf("%s", testo)<0)
>> {la: free(p); return 0;}

>
>If you are going to test the return form printf it would be better to
>test for all errors. Even a non-negative return < strlen(testo) is an
>error.


book say that if printf fail it has to return a <0 value

if testo=="" the string that contain only \0
i think printf has to print nothing to stdin, and return 0
so the above test would be ok in this case too

 
Reply With Quote
 
Rosario1903
Guest
Posts: n/a
 
      02-11-2013
On Mon, 11 Feb 2013 10:16:12 +0100, Rosario1903
<(E-Mail Removed)> wrote:

>i add some error checking...
>What about
>
>#include <stdio.h>
>#include <stdlib.h>
>#include <math.h>
>#include <limits.h>
>
>// ritorna un puntatore da liberarsi tramite free()
>// 0 per errore
>char* GetLineI(char* testo)
>{unsigned s, i, vMax=UINT_MAX-1032; // 1024+4+1 [+ 3]=1032
> int c;
> char *p,*t;
>
> s=1024; p=malloc(s+4);
> if(p==0) return 0;
> if(printf("%s", testo)<0)


better:
if(testo!=0 && printf("%s", testo)<0)


> {la: free(p); return 0;}
> if(fflush(stdout)!=0)
> goto la;
> for(i=0; (c=getchar())!=EOF ; )
> {if(i>=s)
> {if(s>=vMax)
> goto la;
> s=s+1024;
> t=realloc(p, s+4);
> if(t==0)
> goto la;
> p=t;
> }
> p[i]=c; ++i;
> if(c=='\n')
> break;
> }
> if(ferror(stdin))
> goto la;
> p[i]=0;
> return p;
>}
>
>int main(void)
>{char *r;
> r=GetLineI("Inserisci stringa: ");
> if(r==0)
> printf("Errore\n");
> else {printf("Hai inserito: [%s]\n", r);
> free(r);
> }
> return 0;
>}
>


 
Reply With Quote
 
Rosario1903
Guest
Posts: n/a
 
      02-11-2013
On Mon, 11 Feb 2013 16:35:34 +0100, Rosario1903
<(E-Mail Removed)> wrote:


>book say that if printf fail it has to return a <0 value
>
>if testo=="" the string that contain only \0
>i think printf has to print nothing to stdin, and return 0

^^^^^^^
to stdout...

>so the above test would be ok in this case too



 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
LINQ and Entity Framework - Worth a stab? Cirene ASP .Net 6 09-08-2008 01:42 PM
First stab at a weakref test suite Daniel Berger Ruby 6 12-22-2007 07:07 PM
Need camera advice sd memory, aa bat. image stab Bob Digital Photography 3 09-15-2006 08:16 PM
Difference in module_eval taking block vs. taking string (1.8 bug?) Jim Cain Ruby 1 07-18-2003 02:01 AM
ifstream getline() problem John C++ 10 07-14-2003 04:47 PM



Advertisments