Hi All,
I was trying to write a function which will read one line from a
specified file and return the line. It is currently working fine.
But it would be very much helpful for me if some one point out some
improvement points or some error in the code.
Regards,
Somenath
char* get_line(FILE *fp)
{
int len = 2; /*First memory is allocated for 2 bytes 1 for one
character another for '\0'*/
int pos = 0;
char *buff;
char *temp_buff;
char c;
buff = malloc(len);
if (fp == NULL || buff == NULL)
{
return (NULL);
}
while ( (c = fgetc(fp)) != EOF)
{
buff[pos++] = c;
if (c == '\n')
{
/*pos is decremented by one because, currently buff[pos]
contains newline at the
position of pos
but we need to put newline at the end od buf[pos] ;So
pos is decremented by one and
at the end before
return we are assignning '\0' at the place of newline i.e
'\n' is replcaed by '\0'.*/
pos = pos-1;
/*We reached the end of the line*/
break;
}
if (pos == len)
{
len += 2;
temp_buff = realloc(buff,le n);
if (temp_buff == NULL)
{
/*Reallocation of memory fails */
return NULL;
}
else
{
buff = temp_buff;
}
}
}
buff[pos] = '\0';
return buff = (c==EOF?NULL:bu ff);
}
10  1626 
somenath wrote:
Hi All,
I was trying to write a function which will read one line from a
specified file and return the line. It is currently working fine.
But it would be very much helpful for me if some one point out some
improvement points or some error in the code.
Regards,
Somenath
char* get_line(FILE *fp)
{
int len = 2; /*First memory is allocated for 2 bytes 1 for one
character another for '\0'*/
int pos = 0;
char *buff;
char *temp_buff;
char c;
buff = malloc(len);
if (fp == NULL || buff == NULL)
{
return (NULL);
}
while ( (c = fgetc(fp)) != EOF)
{
buff[pos++] = c;
if (c == '\n')
{
/*pos is decremented by one because, currently buff[pos]
contains newline at the
position of pos
but we need to put newline at the end od buf[pos] ;So
pos is decremented by one and
at the end before
return we are assignning '\0' at the place of newline i.e
'\n' is replcaed by '\0'.*/
pos = pos-1;
/*We reached the end of the line*/
break;
}
1)
I think itr would be easier to read to test FIRST for
'\n', THEN add to the buffer if that is the case
if (c == '\n')
break;
buf[pos++] = c;
2)
Under windows, you have new lines with \r \n. You should
test for \r if you want to run under windows and the file
you are reading is opened in binary mode.
if (pos == len)
{
len += 2;
It would be more efficient to reallocate much more than
just 2 characters, to avoid too many calls to
realloc.
temp_buff = realloc(buff,le n);
if (temp_buff == NULL)
{
/*Reallocation of memory fails */
You have a memory leak here. You return without freeing
the old buffer...
return NULL;
}
else
{
buff = temp_buff;
}
}
}
buff[pos] = '\0';
return buff = (c==EOF?NULL:bu ff);
Why return NULL if you hit EOF?
That means that you got a file that doesn't end in a
newline. That is not a BIG SIN. You could just return
what you have read so far.
}
--
jacob navia
jacob at jacob point remcomp point fr
logiciels/informatique http://www.cs.virginia.edu/~lcc-win32
somenath wrote:
>
Hi All,
I was trying to write a function which will read one line from a
specified file and return the line. It is currently working fine.
But it would be very much helpful for me if some one point out some
improvement points or some error in the code.
Regards,
char c;
while ( (c = fgetc(fp)) != EOF)
Right off the bat, (c) should be type int.
--
pete
jacob navia wrote:
>
somenath wrote:
Hi All,
I was trying to write a function which will read one line from a
specified file and return the line. It is currently working fine.
But it would be very much helpful for me if some one point out some
improvement points or some error in the code.
Regards,
Somenath
char* get_line(FILE *fp)
{
int len = 2; /*First memory is allocated for 2 bytes 1 for one
character another for '\0'*/
int pos = 0;
Those int should be size_t.
char *buff;
char *temp_buff;
char c;
buff = malloc(len);
if (fp == NULL || buff == NULL)
{
return (NULL);
}
while ( (c = fgetc(fp)) != EOF)
{
buff[pos++] = c;
if (c == '\n')
{
/*pos is decremented by one because, currently buff[pos]
contains newline at the
position of pos
but we need to put newline at the end od buf[pos] ;So
pos is decremented by one and
at the end before
return we are assignning '\0' at the place of newline i.e
'\n' is replcaed by '\0'.*/
pos = pos-1;
/*We reached the end of the line*/
break;
}
1)
I think itr would be easier to read to test FIRST for
'\n', THEN add to the buffer if that is the case
if (c == '\n')
break;
buf[pos++] = c;
I think so too.
>
2)
Under windows, you have new lines with \r \n. You should
test for \r if you want to run under windows and the file
you are reading is opened in binary mode.
The whole concept of "line" as in "get_line"
applies to streams opened in text mode.
>
if (pos == len)
{
len += 2;
It would be more efficient to reallocate much more than
just 2 characters, to avoid too many calls to
realloc.
How much more?
How do you prevent eventual overflow in (len)?
>
temp_buff = realloc(buff,le n);
if (temp_buff == NULL)
{
/*Reallocation of memory fails */
You have a memory leak here. You return without freeing
the old buffer...
return NULL;
}
else
{
buff = temp_buff;
}
}
}
buff[pos] = '\0';
return buff = (c==EOF?NULL:bu ff);
There's no point in assigning a value to (buff) either,
in the return statement.
Why return NULL if you hit EOF?
That means that you got a file that doesn't end in a
newline.
No.
It could also mean that the get_line function
had read the last character of the file,
on the previous invocation of get_line.
get_line should have some way to indicate
that the end of file has been reached.
Returning NULL is OK.
That is not a BIG SIN. You could just return
what you have read so far.
--
pete
On Nov 5, 5:53 pm, jacob navia <ja...@nospam.o rgwrote:
somenath wrote:
Hi All,
I was trying to write a function which will read one line from a
specified file and return the line. It is currently working fine.
But it would be very much helpful for me if some one point out some
improvement points or some error in the code.
Regards,
Somenath
char* get_line(FILE *fp)
{
int len = 2; /*First memory is allocated for 2 bytes 1 for one
character another for '\0'*/
int pos = 0;
char *buff;
char *temp_buff;
char c;
buff = malloc(len);
if (fp == NULL || buff == NULL)
{
return (NULL);
}
while ( (c = fgetc(fp)) != EOF)
{
buff[pos++] = c;
if (c == '\n')
{
/*pos is decremented by one because, currently buff[pos]
contains newline at the
position of pos
but we need to put newline at the end od buf[pos] ;So
pos is decremented by one and
at the end before
return we are assignning '\0' at the place of newline i.e
'\n' is replcaed by '\0'.*/
pos = pos-1;
/*We reached the end of the line*/
break;
}
1)
I think itr would be easier to read to test FIRST for
'\n', THEN add to the buffer if that is the case
if (c == '\n')
break;
buf[pos++] = c;
2)
Under windows, you have new lines with \r \n. You should
test for \r if you want to run under windows and the file
you are reading is opened in binary mode.
if (pos == len)
{
len += 2;
It would be more efficient to reallocate much more than
just 2 characters, to avoid too many calls to
realloc.
temp_buff = realloc(buff,le n);
if (temp_buff == NULL)
{
/*Reallocation of memory fails */
You have a memory leak here. You return without freeing
the old buffer...
return NULL;
}
else
{
buff = temp_buff;
}
}
}
buff[pos] = '\0';
return buff = (c==EOF?NULL:bu ff);
Why return NULL if you hit EOF?
That means that you got a file that doesn't end in a
newline. That is not a BIG SIN. You could just return
what you have read so far.
}
Many thanks for the information. Actually I wanted same return value
for failure condition i.e NULL .And I wanted to use the this get_line
function as
while((buff= get_line(fp))!= NULL) .If I returned EOF then I did not
find any way to use the get_line function as I wanted.
I would be great help for me if you throw some light so that inspite
of returning EOF still I could use it as I mentioned earlier.
somenath wrote:
Hi All,
I was trying to write a function which will read one line from a
specified file and return the line. It is currently working fine.
But it would be very much helpful for me if some one point out some
improvement points or some error in the code.
Regards,
Somenath
char* get_line(FILE *fp)
{
int len = 2; /*First memory is allocated for 2 bytes 1 for one
character another for '\0'*/
int pos = 0;
char *buff;
char *temp_buff;
char c;
buff = malloc(len);
if (fp == NULL || buff == NULL)
{
return (NULL);
If fp is NULL and the malloc() succeeded, this leaks
the allocated memory.
}
while ( (c = fgetc(fp)) != EOF)
Unreliable test; see the Question 12.1 in the FAQ.
{
buff[pos++] = c;
if (c == '\n')
{
/*pos is decremented by one because, currently buff[pos]
contains newline at the
position of pos
Comment incorrect: buff[pos] contains indeterminate garbage.
buff[pos-1] holds a newline.
but we need to put newline at the end od buf[pos] ;So
pos is decremented by one and
at the end before
return we are assignning '\0' at the place of newline i.e
'\n' is replcaed by '\0'.*/
pos = pos-1;
/*We reached the end of the line*/
break;
}
if (pos == len)
{
len += 2;
temp_buff = realloc(buff,le n);
The parsimonious growth strategy is likely to use a good
deal more compute power than a super-linear strategy would.
if (temp_buff == NULL)
{
/*Reallocation of memory fails */
return NULL;
This leaks the previous buffer.
}
else
{
buff = temp_buff;
}
}
}
buff[pos] = '\0';
return buff = (c==EOF?NULL:bu ff);
This leaks the buffer if EOF was detected. Also, the
assignment is useless.
}
--
Eric Sosman es*****@ieee-dot-org.invalid
On Nov 5, 6:34 pm, pete <pfil...@mindsp ring.comwrote:
jacob navia wrote:
somenath wrote:
Hi All,
I was trying to write a function which will read one line from a
specified file and return the line. It is currently working fine.
But it would be very much helpful for me if some one point out some
improvement points or some error in the code.
Regards,
Somenath
char* get_line(FILE *fp)
{
int len = 2; /*First memory is allocated for 2 bytes 1 for one
character another for '\0'*/
int pos = 0;
Those int should be size_t.
Thanks for the information . But I would request you to explain why
those two variables should be size_t ? According to my knowledge
size_t is type for objects declared to store result of sizeof
operator. Please explain .
>
char *buff;
char *temp_buff;
char c;
buff = malloc(len);
if (fp == NULL || buff == NULL)
{
return (NULL);
}
while ( (c = fgetc(fp)) != EOF)
{
buff[pos++] = c;
if (c == '\n')
{
/*pos is decremented by one because, currently buff[pos]
contains newline at the
position of pos
but we need to put newline at the end od buf[pos] ;So
pos is decremented by one and
at the end before
return we are assignning '\0' at the place of newline i.e
'\n' is replcaed by '\0'.*/
pos = pos-1;
/*We reached the end of the line*/
break;
}
1)
I think itr would be easier to read to test FIRST for
'\n', THEN add to the buffer if that is the case
if (c == '\n')
break;
buf[pos++] = c;
I think so too.
2)
Under windows, you have new lines with \r \n. You should
test for \r if you want to run under windows and the file
you are reading is opened in binary mode.
The whole concept of "line" as in "get_line"
applies to streams opened in text mode.
if (pos == len)
{
len += 2;
It would be more efficient to reallocate much more than
just 2 characters, to avoid too many calls to
realloc.
How much more?
How do you prevent eventual overflow in (len)?
temp_buff = realloc(buff,le n);
if (temp_buff == NULL)
{
/*Reallocation of memory fails */
You have a memory leak here. You return without freeing
the old buffer...
return NULL;
}
else
{
buff = temp_buff;
}
}
}
buff[pos] = '\0';
return buff = (c==EOF?NULL:bu ff);
There's no point in assigning a value to (buff) either,
in the return statement.
Why return NULL if you hit EOF?
That means that you got a file that doesn't end in a
newline.
No.
It could also mean that the get_line function
had read the last character of the file,
on the previous invocation of get_line.
get_line should have some way to indicate
that the end of file has been reached.
Returning NULL is OK.
That is not a BIG SIN. You could just return
what you have read so far.
somenath wrote:
On Nov 5, 6:34 pm, pete <pfil...@mindsp ring.comwrote:
>jacob navia wrote:
>>somenath wrote:
Hi All,
I was trying to write a function which will read one line from a
specified file and return the line. It is currently working fine.
But it would be very much helpful for me if some one point out some
improvemen t points or some error in the code.
Regards,
Somenath
char* get_line(FILE *fp)
{
int len = 2; /*First memory is allocated for 2 bytes 1 for one
character another for '\0'*/
int pos = 0;
Those int should be size_t.
Thanks for the information . But I would request you to explain why
those two variables should be size_t ? According to my knowledge
size_t is type for objects declared to store result of sizeof
operator. Please explain .
What is the prototype for malloc()?
somenath wrote, On 05/11/07 12:43:
Hi All,
I was trying to write a function which will read one line from a
specified file and return the line. It is currently working fine.
But it would be very much helpful for me if some one point out some
improvement points or some error in the code.
Regards,
Somenath
char* get_line(FILE *fp)
{
int len = 2; /*First memory is allocated for 2 bytes 1 for one
character another for '\0'*/
int pos = 0;
char *buff;
char *temp_buff;
char c;
buff = malloc(len);
if (fp == NULL || buff == NULL)
{
return (NULL);
}
You have a memory leak if fp was NULL and malloc succeeded. Test fp
before doing the malloc.
while ( (c = fgetc(fp)) != EOF)
c should have been declared as an int rather than a char. If char is
unsigned this test will not detect EOF and if char is signed at least
one potentially valid character could be detected as EOF.
{
buff[pos++] = c;
if (c == '\n')
{
/*pos is decremented by one because, currently buff[pos]
contains newline at the
position of pos
but we need to put newline at the end od buf[pos] ;So
pos is decremented by one and
at the end before
return we are assignning '\0' at the place of newline i.e
'\n' is replcaed by '\0'.*/
pos = pos-1;
One of "pos -= 1", "pos--" or "--pos" would be clearer in my opinion.
/*We reached the end of the line*/
break;
}
if (pos == len)
{
len += 2;
temp_buff = realloc(buff,le n);
if (temp_buff == NULL)
{
/*Reallocation of memory fails */
This is a memory leak since buf still points to allocated memory.
return NULL;
}
else
{
buff = temp_buff;
}
}
}
buff[pos] = '\0';
return buff = (c==EOF?NULL:bu ff);
This is another memory leak when the end of file is reached or a an
error occurs.
}
In general you should not throw away the successfully read data if an
error occurs. You should return what you have and also indicate that a
failure has occurred. It is also subject to a DoS (Denial of Service)
attack by feeding it a continuous stream or data that does not include a
new line.
--
Flash Gordon
On Nov 5, 6:56 pm, Mark Bluemel <mark_blue...@p obox.comwrote:
somenath wrote:
On Nov 5, 6:34 pm, pete <pfil...@mindsp ring.comwrote:
jacob navia wrote:
>somenath wrote:
Hi All,
I was trying to write a function which will read one line from a
specified file and return the line. It is currently working fine.
But it would be very much helpful for me if some one point out some
improvement points or some error in the code.
Regards,
Somenath
char* get_line(FILE *fp)
{
int len = 2; /*First memory is allocated for 2 bytes 1 for one
character another for '\0'*/
int pos = 0;
Those int should be size_t.
Thanks for the information . But I would request you to explain why
those two variables should be size_t ? According to my knowledge
size_t is type for objects declared to store result of sizeof
operator. Please explain .
What is the prototype for malloc()?- Hide quoted text -
Many thanks . I got the answer. This thread has been closed and replies have been disabled. Please start a new discussion. Similar topics | |
by: Cardman |
last post by:
Greetings,
I am trying to solve a problem that has been inflicting my self
created Order Forms for a long time, where the problem is that as I
cannot reproduce this error myself, then it is difficult to know what
is going on.
One of these Order Forms you can see here...
http://www.cardman.co.uk/orderform.php3
|
by: mike |
last post by:
Hello,
After trying to validate this page for a couple of days now I was
wondering if someone might be able to help me out. Below is a list of
snippets where I am having the errors.
1. Line 334, column 13: there is no attribute "SRC"
<bgsound src="C:\My Documents\zingwent.mids">
You have used the attribute named above in your document, but the document
type you are using does not support that attribute for this element. This
error is...
|
by: futurepy |
last post by:
Hello,
I am not good at JS at all. If you are reading this message with the
IE, please move the cursor up to the top of the page. When the cursor
stops at an icon, a rectangle slot will appear, and there is text in
the slot, saying 'Back to ...', 'Stop', 'Home', 'Refresh', etc.
This is what I like to learn. In my own web page, when the reader
moves the cursor to a word, sentence or URL address, I would like to
have a rectangle slot...
|
by: Marquisha |
last post by:
If this is off-topic, please forgive me. But I thought this might be
the perfect spot to get some advice about how to proceed with a
project.
Working on a Web site design for a nonprofit organization, and I'm
donating the work. Haven't done Web work in a while, and things have
changed since I last did anything of this magnitude. I'm looking for
a solution that will enable me to edit pages easily and in totally
WYSIWYG fashion, while...
|
by: Sori Schwimmer |
last post by:
0) Sorry, I don't know how to post a reply in the same
thread.
1) Grant Edwards wrote:
> The "i += 1" line is almost certainly wrong.
You're certainly write, as I acknowledged in a follow
up "suggestion for (re)try statement - correction'
2) Rocco Morreti wrote:
> What is so repugnant about the equivalent, currently
valid way of writing it?
Nothing "repugnant". We have in almost all procedural
| | |
by: Michael B. |
last post by:
I was just thinking about this, specifically wondering if there's any
features that the C specification currently lacks, and which may be
included in some future standardization.
Of course, I speak only of features in the spirit of C; something like
object-orientation, though a nice feature, does not belong in C.
Something like being able to #define a #define would be very handy,
though, e.g:
#define DECLARE_FOO(bar) #define...
|
by: Jim |
last post by:
I am trying to write an HTTP/HTTPS proxy server in VB.Net 2005.
But, I don't really even know how the internal workings of a proxy should
act. Does anyone have anything on the protocols used in handling requests by
proxies?
I have Googled my eyes out....and found nothing.
(BTW, props to Google for refusing the ridiculous request for their search
results.)
|
by: Justin Weinberg |
last post by:
My thoughts on this....
http://msdn.microsoft.com/vbasic/Future/default.aspx?pull=/library/en-us/dnvs05/html/vb9overview.asp
My thoughts:
1. Regarding Implicit types, I don't use type declaration for the benefits
of performance. It's for the benefit of clarity of purpose when reading
code. The first thing I do with neophyte developers is turn Option Strict
|
by: Rafael Tejera |
last post by:
I'm moving some code from my old pc. I'm using VS NET 2003 C# and MSSQL 2003. Everything is in place, but I'm receiving this error messages.
thansk for you help...
Here is the error message:
Server Error in '/' Application.
|
by: Oralloy |
last post by:
Hello folks,
I am unable to find appropriate documentation on the type promotion of bit-fields when using the generalised comparison operator "<=>".
The problem is that using the GNU compilers, it seems that the internal comparison operator "<=>" tries to promote arguments from unsigned to signed.
This is as boiled down as I can make it.
Here is my compilation command:
g++-12 -std=c++20 -Wnarrowing bit_field.cpp
Here is the code in...
|
by: jinu1996 |
last post by:
In today's digital age, having a compelling online presence is paramount for businesses aiming to thrive in a competitive landscape. At the heart of this digital strategy lies an intricately woven tapestry of website design and digital marketing. It's not merely about having a website; it's about crafting an immersive digital experience that captivates audiences and drives business growth.
The Art of Business Website Design
Your website is...
| | |
by: Hystou |
last post by:
Overview:
Windows 11 and 10 have less user interface control over operating system update behaviour than previous versions of Windows. In Windows 11 and 10, there is no way to turn off the Windows Update option using the Control Panel or Settings app; it automatically checks for updates and installs any it finds, whether you like it or not. For most users, this new feature is actually very convenient. If you want to control the update process,...
|
by: agi2029 |
last post by:
Let's talk about the concept of autonomous AI software engineers and no-code agents. These AIs are designed to manage the entire lifecycle of a software development project—planning, coding, testing, and deployment—without human intervention. Imagine an AI that can take a project description, break it down, write the code, debug it, and then launch it, all on its own....
Now, this would greatly impact the work of software developers. The idea...
|
by: isladogs |
last post by:
The next Access Europe User Group meeting will be on Wednesday 1 May 2024 starting at 18:00 UK time (6PM UTC+1) and finishing by 19:30 (7.30PM).
In this session, we are pleased to welcome a new presenter, Adolph Dupré who will be discussing some powerful techniques for using class modules.
He will explain when you may want to use classes instead of User Defined Types (UDT). For example, to manage the data in unbound forms.
Adolph will...
|
by: conductexam |
last post by:
I have .net C# application in which I am extracting data from word file and save it in database particularly. To store word all data as it is I am converting the whole word file firstly in HTML and then checking html paragraph one by one.
At the time of converting from word file to html my equations which are in the word document file was convert into image.
Globals.ThisAddIn.Application.ActiveDocument.Select();...
|
by: TSSRALBI |
last post by:
Hello
I'm a network technician in training and I need your help.
I am currently learning how to create and manage the different types of VPNs and I have a question about LAN-to-LAN VPNs.
The last exercise I practiced was to create a LAN-to-LAN VPN between two Pfsense firewalls, by using IPSEC protocols.
I succeeded, with both firewalls in the same network. But I'm wondering if it's possible to do the same thing, with 2 Pfsense firewalls...
|
by: adsilva |
last post by:
A Windows Forms form does not have the event Unload, like VB6. What one acts like?
| | |
by: 6302768590 |
last post by:
Hai team
i want code for transfer the data from one system to another through IP address by using C# our system has to for every 5mins then we have to update the data what the data is updated we have to send another system
| |
