0

I am trying to read in a cstring from a edit control box in MFC, then put it into a char array in a struct, but since I cannot do something like clientPacket->path = convertfuntion(a); I had to create another char array to store the string then store it element by element.

That felt like a bandait solution, is there a better way to approach this? I'd like to learn how to clean up the code.

CString stri;//Read text from edit control box and convert it to std::string
GetDlgItem(IDC_EDIT1)->GetWindowText(stri);
string a;
a = CT2A(stri);
char holder[256];

strcpy_s(holder,a.c_str());
int size = sizeof(holder);

struct packet {
    char caseRadio;
    char path[256];
};
packet* clientPacket = new packet;
for (int t = 0; t < size; t++) {
    clientPacket->path[t] = holder[t] ;
}

EDIT:This is currently what I went with:

CString stri;//Read text from edit control box and convert it to std::string
GetDlgItem(IDC_EDIT1)->GetWindowText(stri);
string a = CT2A(stri);

struct packet {
    char caseRadio;
    char path[CONSTANT];//#define CONSTANT 256
};
packet* clientPacket = new packet;

a = a.substr(0, sizeof(clientPacket->path) - 1);
strcpy_s(clientPacket->path, a.c_str());

I got a problem where I got "1path" instead of "path", turns out it read in caseRadio='1', fixed it by reading out caseRadio first in the server

Mochideer
  • 35
  • 8
  • 2
    Why not copy straight into `path`? Note that `sizeof(holder)` will always be `256` you probably meant `strlen(holder)` – Alan Birtles Jan 17 '22 at 06:43
  • 1
    There's a `CT2A` conversion macro in there. Do you need that conversion? Regardless of that, you can simply call [`GetWindowTextA`](https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-getwindowtexta) (the Windows API call, not the MFC wrapper) to directly copy into `packet::path`. – IInspectable Jan 17 '22 at 06:51

4 Answers4

1

I don't see the need to create the intermediate 'holder' char array. I think you can just directly do

strcpy(clientPacket->path, a.c_str());

You may want to do this:

a= a.substr(0, sizeof(clientPacket->path)-1);

before the strcpy to avoid buffer overrun depending on whether the edit text is size limited or not.

Sami Sallinen
  • 3,203
  • 12
  • 16
  • I get a "1recieved" instead of "recieved" on my server, where does the 1 come from? – Mochideer Jan 17 '22 at 07:19
  • 1
    That's unfortuantely impossible to say without looking at your program more closely. I e.g. don't know what the CT2A macro does. I suggest you use a debugger to inspect the variables before the packet is sent. If you can't use a debugger, print the different variables to stdout or to a log file, that way you should be able to figure out the stage at which the 1 gets inserted. – Sami Sallinen Jan 17 '22 at 07:25
  • Got it, I will try to figure it out myself thx :) – Mochideer Jan 17 '22 at 07:34
  • Doing this still has 2 (or 3) needless (and expensive) heap allocations. First, the `CString` to copy into. The next allocation when constructing a `std::string` after the possible conversion. And finally the allocation of *another* `std::string` when requesting the `substr`. The actual solution requires zero heap allocations (or one, if you need to extend the lifetime of `packet` beyond its scope). – IInspectable Jan 17 '22 at 08:10
1

You can copy directly into a user-provided buffer when using the Windows API call GetWindowTextA. The following illustrates how to do this:

struct packet {
    char caseRadio;
    char path[512];
} p;

::GetWindowTextA(GetDlgItem(IDC_EDIT1)->GetSafeHwnd(), &p.path[0],
                 static_cast<int>(sizeof(p.path)));

This does an implicit character encoding conversion using the CP_ACP code page. This is not generally desirable, and you may wish to perform the conversion using a known character encoding (such as CP_UTF8).

IInspectable
  • 46,945
  • 8
  • 85
  • 181
  • I tried HWND Handle = NULL; GetDlgItem(IDC_EDIT1, &Handle); ::GetWindowTextA(Handle, &p.path[0],static_cast(sizeof(p.path))); But it is now passing weird values – Mochideer Jan 17 '22 at 08:09
  • btw what does :: mean here? – Mochideer Jan 17 '22 at 08:10
  • 1
    There's no way of knowing why you would observe weird values. A debugger will help you understand what's going on. My guess: You're returning the address of a local variable. The meaning of `::` is explained in [this Q&A](https://stackoverflow.com/q/75213/1889329). – IInspectable Jan 17 '22 at 08:19
0

Use the CString.GetBuffer function to get a pointer to the string. In your struct, store the path as a char* instead of a char array.

struct packet {
    char caseRadio;
    char* path;
};
packet* clientPacket = new packet;
clientPacket->path = stri.GetBuffer();
Frost.
  • 109
  • 1
  • 1
  • 4
  • 2
    When `stri` goes out of scope you now have a dangling pointer in `packet`. Unlikely to be a desirable solution. – IInspectable Jan 17 '22 at 06:48
  • Seems the destructor of CString does not deallocate memory. – Frost. Jan 17 '22 at 06:50
  • Why would it not? Of course it does. Practically **all** MFC types manage their resources, and `CStringT` is certainly not an exception. – IInspectable Jan 17 '22 at 06:52
  • I am sending the struct through a socket connection, from my previous attempt it seems like there cannot be a pointer in a struct to recieve data. – Mochideer Jan 17 '22 at 06:52
  • 1
    Then just copy the string to your struct directly. strcpy(clientPacket->path, stri.GetBuffer()) – Frost. Jan 17 '22 at 06:57
0

Like this, maybe? strncpy(clientPacket->path, CT2A(stri).c_str(), 255);. Also, better make the 256 bytes a constant and use that name, just in case you change this in 10 years.

KungPhoo
  • 516
  • 4
  • 18