Fix: Improve handling of very large arguments in the tracebld sample#83
Fix: Improve handling of very large arguments in the tracebld sample#83ScatteredRay wants to merge 3 commits intomicrosoft:mainfrom
Conversation
…, we end up skipping past the beginning of the argument causing the rest to fail.
…uffer, we just send it over the line in fragments instead.
ScatteredRay
force-pushed
the
bldtrace_arg_fixes
branch
from
97c3684 to
4cb773a
Compare
ScatteredRay
left a comment
ScatteredRay
left a comment
There was a problem hiding this comment.
Added some comments about what this fixes.
| WCHAR wzPath[MAX_PATH]; | ||
| PWCHAR pwzPath = wzPath; | ||
| PCWSTR pwzTmp = pwzArgBeg + 1; | ||
| PCWSTR pwzTmp = pwzArgBeg; |
There was a problem hiding this comment.
This is a fix for an off-by-one error in response file parsing. Response file parsing doesn't work at all without this fix.
| Tblog("<t:Line>%le %le</t:Line>\n", wzPath, pwzFin); | ||
| Tblog("<t:Line>%le ", wzPath); | ||
| } | ||
| for(PWCHAR pwzTmp = pwzFin; pwzTmp < pwzFin + wcNew; pwzTmp += 32764) { |
There was a problem hiding this comment.
if pwzTmp is very large, it overflows the structure that passes this string to the host app. This solution sends it in parts.
There was a problem hiding this comment.
I don't think this does what you think it does. The problem is that the characters in the string are being escaped. Some input characters require 6 characters in the output. This means that the output may require more than 32764 characters. if it does, some characters in the input string may not be written.
It is also not a good idea to embed a constant like 32764 here. This constant comes from tracebld.h, specifically the number of characters in szMessage in the definition of the struct _TBLOG_MESSAGE. In fact, looking at the code for VSafePrintf, it looks like one character is reserved for a null terminator (line 1880 in trcbld.cpp).
dtarditi
left a comment
dtarditi
left a comment
There was a problem hiding this comment.
@ScatteredRay, thank for the fixes. The first fix for the off-by-one looks good. I think you need to revise how a large string is being broken into chunks.
| Tblog("<t:Line>%le %le</t:Line>\n", wzPath, pwzFin); | ||
| Tblog("<t:Line>%le ", wzPath); | ||
| } | ||
| for(PWCHAR pwzTmp = pwzFin; pwzTmp < pwzFin + wcNew; pwzTmp += 32764) { |
There was a problem hiding this comment.
I don't think this does what you think it does. The problem is that the characters in the string are being escaped. Some input characters require 6 characters in the output. This means that the output may require more than 32764 characters. if it does, some characters in the input string may not be written.
It is also not a good idea to embed a constant like 32764 here. This constant comes from tracebld.h, specifically the number of characters in szMessage in the definition of the struct _TBLOG_MESSAGE. In fact, looking at the code for VSafePrintf, it looks like one character is reserved for a null terminator (line 1880 in trcbld.cpp).
|
@ScatteredRay Do you think you'll get some time to address @dtarditi's feedback? |
bgianfo
added
the
needs-author-feedback
|
Ping @ScatteredRay |
|
Hey bgianfo. I do want to be able to address this and create a proper fix. I think I understand the issue, but aspects about it, specifically, the details about escaped characters will make it a bit more involved, and I'm both a bit busy with other work right now, and not depending on these fixes currently. So, it's on my backlog, but I don't have a sense of when I will get around to the problem. |
ghost
removed
the
needs-author-feedback
bgianfo
added
the
needs-author-feedback
|
This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. |
bgianfo
changed the title
|
This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. |
3 participants
There are two fixes here, the first fixes an off by one error in response file parsing that causes parsing it to fail. The second is an improvement to handle command line arguments that go over int16_max including xml tags and message padding.