Detecting Garbage chars in CString

[hypheni]

How do I detect garbage chars in a CString. Actually I'm reading some data from COM port. In some certain condition it will give some garbage as a version no. Now I need to show _T("N/A") in case of there is any garbage.

My solution is to check for a Valid char or integer. If found its correct else Garbage.

Any other way?

[VictorN]

Rather than "detect garbage chars in a CString" you would better not put them in the CString!
Note that CString in not a good means to collect data from a serial port. Better choice would be a CByteArray because port receives bytes, not strings!
Or you have to use CString (better would be CStringA) very carefully to terminate its content with NULL after receiving will have finished.

[hypheni]

Actually I'm reading from COM port in BYTE array and then casting it to CString for printing in Log where it is printing value: 0 0 4 131

[VictorN]

And how do you cast BYTE array to CString? Could you show the code?

[hypheni]

  BYTE ucData[60];

  for(int i = 0;i<60;i++)
    ucData[i] = 0;

  short iNoOfReadByte = (short)m_DsiSerialComm.ReadCommPort(ucData, 60);
  CString cstrReadData = _T("");
  CString cstrTemp = _T("");
  for(int loop = 1; loop < iNoOfReadByte; loop++)
  {
    cstrTemp = _T("");
    cstrTemp.Format(_T("%c"), ucData[loop]);
    cstrReadData = cstrReadData + cstrTemp;
    if(ucData[loop + 1] == '\n')
    {
      break;
    }
  }

  //Log cstrReadData here.

[VictorN]

Well, a lot of "actions" that are NOT needed at all...
So I'd first overwrite your code to make it shorter and more descriptive:

  BYTE ucData[60] = {0};  // initialize all the elements with zeros

  short iNoOfReadByte = (short)m_DsiSerialComm.ReadCommPort(ucData, 60);
  CString cstrReadData;   // default ctor creates an empty string!
  for(int loop = 1; loop < iNoOfReadByte; loop++)
  {
    CString cstrTemp;   // default ctor creates an empty string!
    cstrTemp.Format(_T("%c"), ucData[loop]);
    cstrReadData = cstrReadData + cstrTemp;
    if(ucData[loop + 1] == '\n')
    {
      break;
    }
  }

Now there are my questions/opinions:

1. what does m_DsiSerialComm.ReadCommPort() return? the number of bytes read from a port?
2. why do you begin your loop from 1 rather than 0? Why do you omit the first byte?
3. you compare with the '\n' the array element next to the loop index, thus being at the last array position (element 59) you access something beyond this array! As a result you get (in the best variant) an undefined behavior or (worse) an access violation!

[Paul McKenzie]

Actually I'm reading from COM port in BYTE array and then casting it to CString for printing in Log where it is printing value: 0 0 4 131

CStrings are not BYTE arrays.

A CString is a class that handles either ANSI or UNICODE strings, depending on the build type. This means that the character size is either going to be 1 or 2 bytes per character, again depending on the build. A BYTE array is the same thing, regardless of the build. It will always store BYTE data.

Anytime you're casting things to a CString, this is almost always wrong. Whatever you're casting to a CString has to adjust to fit CString's type, and CString's underlying type varies depending on the build settings.

Regards,

Paul McKenzie

[2kaud]

You might try something like this

#define BUFFSIZE 60

BYTE ucData[BUFFSIZE + 1] = {0};

CString cstrReadData;

     m_DsiSerialComm.ReadCommPart(ucData, BUFFSIZE);
     cstrReadData = ucData;
     cstrReadData.Replace('\n', 0);

[hypheni]

Well the code is buggy as I asked to the original author.

Ans.
1. Yes
2. 0 index holds some discard able data
3. Well, I omitted that line and working same, but it is the indicator for the end of required data. And I changed it to
if(loop != 59 && ucData[loop + 1] == '\n')
{
break;
}

[VictorN]

You might try something like this

#define BUFFSIZE 60

BYTE ucData[BUFFSIZE + 1] = {0};

CString cstrReadData;

     m_DsiSerialComm.ReadCommPart(ucData, BUFFSIZE);
     cstrReadData = ucData;
     cstrReadData.Replace('\n', 0);

It is not correct for UNICODE build.
Nor will it correct using CStringA instead of CString! Just consider the case with more than one '\n' in the ucData buffer!

[2kaud]

Just consider the case with more than one '\n' in the ucData buffer!

In the original code, the concatenation stops when the first '\n' is found so subsequent data in ucData are not processed.

It is not correct for UNICODE build.

The following test program gives the expected output of

qwerty

#define UNICODE
#include <atlstr.h>
using namespace std;

int main()
{
unsigned char ch1[60] = "qwerty\nsdfgzxcvb";

     CString cs1 = ch1;
     cs1.Replace('\n', 0);
     _putws((LPCWSTR)cs1);
     return (0);
}

[VictorN]

In the original code, the concatenation stops when the first '\n' is found so subsequent data in ucData are not processed.

You are right. But it is just one more bug in OP's code!

The following test program gives the expected output of

qwerty

#define UNICODE
#include <atlstr.h>
using namespace std;

int main()
{
unsigned char ch1[60] = "qwerty\nsdfgzxcvb";

     CString cs1 = ch1;
     cs1.Replace('\n', 0);
     _putws((LPCWSTR)cs1);
     return (0);
}

But it should be

     cs1.Replace(_T('\n'), 0);