using fgets() to read comport in C/C++

Question

1.00/5 (2 votes)

See more:

I'm trying write a program to send commands over the comport and receive a response. At the moment I can open/close and write to the comport but can't get fgets() to work reading a line in from the comport. At the moment the program crashes after sending the string to the comport. I am using windows (visual studio 2008).

code below

Thanks

C++

#include "stdafx.h"
#include <string.h>
#include <conio.h>
#include <stdlib.h>

int main (void)

{
	char str[80];
	char rstr[80];
	char buff[256];

	FILE *comport;
	if ((comport = fopen("COM1", "wt")) == NULL)
		{
			printf("Failed to open the communication port COM1\n");
			printf("The port may be disabled or in use\n");
			printf("enter \"quit\" to exit \n");
			getch();
		}
	else
		{
		printf("COM1 opended successfully\n");
		
			for(;;)
			{
				printf ("enter a string \n");
				fgets (str , 80 , stdin);						
				printf ("\n%s\n",str);
				fprintf (comport, "%s\n",str);
				fflush(comport);

				fgets(buff, sizeof buff, comport);  
				
				strcpy(rstr, buff );
				printf("s%\n",rstr);
											
			}
		fclose(comport);
		

		}
	return 0;
}

Posted 19-Aug-12 14:56pm

Member 8526422

Updated 19-Aug-12 15:14pm

Wes Aday

v3

Add a Solution

Comments

Sergey Alexandrovich Kryukov 19-Aug-12 21:56pm

"Crashed"? Are you a software developer or not? Run it under debugger or catch all the exceptions to provide exact exception information. First of all, indicate the line of code where an exception is thrown.
--SA

pasztorpisti 19-Aug-12 22:14pm

:-D Not using the debugging facilities of the best c++ ide available is really a guilt.

Sergey Alexandrovich Kryukov 19-Aug-12 22:55pm

True.

nv3 20-Aug-12 5:48am

Fully agree! Some people seem to quit when it comes to debugging. Some 30 percent of all posts here deal with situations where people simply didn't know how to use a debugger. What a tragedy.

pasztorpisti 20-Aug-12 7:38am

Debugging can also be tremendous fun. Its another "game mode" in software development. :-)

nv3 20-Aug-12 7:47am

And not only fun, we use the internal iron rule: Never release a part of code that you have not single-stepped through. And we have caught sooo many bugs by that, even before they turned up in a bug report.

Sergey Alexandrovich Kryukov 20-Aug-12 15:17pm

Certainly, single-stepped is a huge exaggeration, a figure of speech, I presume :-)
The better technology we have, the greater is the portion of code which runs correctly from the first attempt, really.
--SA

Sergey Alexandrovich Kryukov 20-Aug-12 15:15pm

Tragedy?! :-) Right. The typical feature of the classic tragedy is that by the finale Deus ex machina comes to enforce the justice. Is that something our typical hero is looking for? :-)

In real life, the hero more usually acts on her/his own or otherwise fails even before the performance appears on a public scene...
--SA

P. S.: For those who sit in tanks :-): http://en.wikipedia.org/wiki/Deus_ex_machina.

2 solutions

Solution 2

This is old, but just in case anyone else is looking at this... There are numerous bugs in the posted code.

1) Buffer Overflow risk (high):

C#

strcpy(rstr, buff );

Because buff is bigger than rstr

2) An endless for(;;) loop. Use while(!feof(stdin) && !feof(comport)) so that you can break out and hit the fclose when one of the streams is at EOF.

3) When the COM port is busy you don't need to enter "quit" to exit, getch() just waits for a keypress.

4) Put a return 0 after the getch() so that you don't need an else block.

5) As has been pointed out, check the return values from fgets. If nullptr you need to drop out of the loop (continue will do it if you follow #2.)

6) I highly suggest using std::array<char,> instead of char [] buffers. Along with strcpy_s or sprintf_s, using the .size() member of the array, you will be at less risk of overflows.

And, not to pile on, but that is a weird coding style. ;)

Posted 13-Jun-14 7:18am

liahona

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

pasztorpisti · Accepted Answer · 2012-08-19T15:56:00

Solution 1

You should check the return value of fgets(), I guess it would be NULL because you opened the com port only for writing. After the unsuccessful fgets the next 2 lines can easily crash. Instead of "wt" try "r+" as file mode to open com1 for both reading and writing and check the return value of fgets.

Posted 19-Aug-12 15:56pm

pasztorpisti

Comments

Sergey Alexandrovich Kryukov 19-Aug-12 22:55pm

5ed.
--SA

pasztorpisti 20-Aug-12 7:36am

Thank you!

Member 8526422 20-Aug-12 19:39pm

Thank for Ponting out the fopen mistake, had not seen that at all, now corrected (r+). I have changed the code to look for the return value of fgets and tried some debugging but still can not read the comport. The return value of fgets is still always zero, his is where to program hangs. I have tried replacing the read comport to read from stdin (you can see a previous while condition commented out) and this works.

code modification below

while (true) //(fgets(buff, sizeof buff, stdin)== NULL);
{
if(fgets(buff, sizeof buff, comport)!= NULL);
{
//strcpy(rstr, buff );
printf("%s\n",buff);
break;
}
}

pasztorpisti 20-Aug-12 20:19pm

Since you are using crt functions you could check the value of errno after fgets. if that doesnt help then god knows what the problem is. I worked with com ports only once, long ago, and used windows specific api for that. If you give up using fgets() and its friend functions then search for some windows com port programming tutorials with google. That will work for sure, it doesnt worth wasting your time with fgets if it doesn work.