Bug #1724
LocalProcess portability issue: LocalProcess::Read2 can not return more than 1024B on PLATFORM_WIN but it can on POSIX
Status: | Approved | Start date: | 05/17/2017 | |
---|---|---|---|---|
Priority: | Normal | Due date: | ||
Assignee: | Abdelghani Omari | % Done: | 0% | |
Category: | Core | Spent time: | - | |
Target version: | - |
Description
test case:
CONSOLE_APP_MAIN { String str = String('A', 1024); str << "B"; String f = GetDataFile("data.txt"); SaveFile(f, str); String cmd; #ifdef PLATFORM_POSIX cmd << "cat \"" << f << "\""; #else cmd << "cmd /c Type \"" << f << "\""; #endif LocalProcess proc; proc.Start(cmd); Sleep(200); String r; proc.Read(r); DUMP(r.GetCount()); proc.Read(r); DUMP(r.GetCount()); }
output :
POSIX:
r.GetCount() = 1025 r.GetCount() = 0
WIN32:
r.GetCount() = 1024 r.GetCount() = 1
this is because the WIN32 version use "if" and the POSIX use "while".
the patch :
bool LocalProcess::Read2(String& reso, String& rese) { LLOG("LocalProcess::Read2"); reso = wreso; rese = wrese; wreso.Clear(); wrese.Clear(); #ifdef PLATFORM_WIN32 LLOG("LocalProcess::Read"); bool was_running = IsRunning(); char buffer[1024]; dword n; while(hOutputRead && PeekNamedPipe(hOutputRead, NULL, 0, NULL, &n, NULL) && n && // LINE modified : replace "if" with " "while" ReadFile(hOutputRead, buffer, sizeof(buffer), &n, NULL) && n) reso.Cat(buffer, n); while(hErrorRead && PeekNamedPipe(hErrorRead, NULL, 0, NULL, &n, NULL) && n && // LINE modified : replace "if" with " "while" ReadFile(hErrorRead, buffer, sizeof(buffer), &n, NULL) && n) rese.Cat(buffer, n); if(convertcharset) { reso = FromOEMCharset(reso); rese = FromOEMCharset(rese); } return .......
History
#1 Updated by Miroslav Fidler over 7 years ago
- Assignee changed from Miroslav Fidler to Abdelghani Omari
I do not think this is an error. Contract is that Read returns "some" data if available. You are supposed to call it again for the next chunk.
Even in this example, reading the output of "ls" can return less and then some more on the next call, as there might not be data in the pide yet.
In fact, if something needs fixing here, it is perhaps posix version - if some process would be producing a lot of data quickly, it would produce very large Strings, which would be bad in some cases. I guess there should be "if" too...
Mirek
#2 Updated by Abdelghani Omari over 7 years ago
Ok, "if" for posix version resolve the portability issue.
#3 Updated by Abdelghani Omari over 7 years ago
- Assignee changed from Abdelghani Omari to Miroslav Fidler
#4 Updated by Miroslav Fidler over 7 years ago
- Status changed from Patch ready to Ready for QA
- Assignee changed from Miroslav Fidler to Abdelghani Omari
It is now 'if' in POSIX Read...
#5 Updated by Abdelghani Omari over 7 years ago
- Status changed from Ready for QA to Approved