Closed
Bug 482255
Opened 15 years ago
Closed 15 years ago
need remote test harness for windows ce
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: blassey, Assigned: blassey)
Details
(Keywords: mobile)
Attachments
(1 file, 3 obsolete files)
17.81 KB,
patch
|
dougt
:
review+
|
Details | Diff | Splinter Review |
We should be able to do this with CeCreateProcess passing in DEBUG_PROCESS to get a handle to stdout. That's the plan anyway.
Flags: wanted-fennec1.0?
Assignee | ||
Comment 1•15 years ago
|
||
cestub.dll needs to be installed in the wince path (either \ or \Windows). ceexec.exe takes the command line to execture (absolute path and args).
Attachment #366489 -
Flags: review?(doug.turner)
Assignee | ||
Comment 2•15 years ago
|
||
Turns out that IRAPIStream::Write won't actually write the data if less than 4 bytes is passed in.
Attachment #366489 -
Attachment is obsolete: true
Attachment #366758 -
Flags: review?(doug.turner)
Attachment #366489 -
Flags: review?(doug.turner)
Comment 3•15 years ago
|
||
Comment on attachment 366758 [details] [diff] [review] fixes returning exit code nice try blassey.
Attachment #366758 -
Flags: review?(doug.turner) → review-
Assignee | ||
Comment 4•15 years ago
|
||
oops, sorry note though that the .def file gives warnings such as these: c:/hg/mc-min/build/wince/remote/client/cestub.def(1) : warning LNK4017: # statement not supported for the target platform; ignored That is for the license block. Ignoring is what we want it to do, but its noisy.
Attachment #366758 -
Attachment is obsolete: true
Attachment #366817 -
Flags: review?(doug.turner)
Comment 5•15 years ago
|
||
(In reply to comment #4) > note though that the .def file gives warnings such as these: > c:/hg/mc-min/build/wince/remote/client/cestub.def(1) : warning LNK4017: # > statement not supported for the target platform; ignored > > That is for the license block. Ignoring is what we want it to do, but its > noisy. You presumably can use semicolon(;) for comment line in def file.
Comment 6•15 years ago
|
||
Comment on attachment 366817 [details] [diff] [review] an actual patch +DIRS += wince/shunt \ + wince/remote/client \ + wince/remote/host \ + $(NULL) nit: These should be aligned. +# Portions created by the Initial Developer are Copyright (C) 2___ Fix the year + +TOPSRCDIR = $(topsrcdir) +OBJDIR = $(DEPTH) + +export NO_SHUNT = 1 Could you add a comment as to why you need these. + * Brad Lassey <blassey@moilla.com> missing a 'z' +int __cdecl RemoteExec(DWORD dwInput, BYTE* pInput, + DWORD* ret, BYTE** ppOutput, + IRAPIStream* pStream) Align the params, and there are two spaces between int and __cdecl ooc, do you need to explictly dllexport when using a def file? + space = wcschr(quote2, ' '); + + } else { nit: Ditch the new line + switch(de.dwDebugEventCode) { + case EXCEPTION_DEBUG_EVENT: nit: space the case statements in (they should be under the switch) + de.dwProcessId = pi.dwProcessId; + de.dwThreadId = pi.dwThreadId; Does this matter? I think WaitForDebugEvent fills these in, and you need to check, when WaitForDebugEvent returns from blocking, if the thread and process ID match what you expect. Maybe for this app, you want to print every event, so just ditch the initial assignments. + goto RETURN; no religous debate here, but why not just have a keepGoing variable that we set, and have your for(;;) statement turn into while(keepGoing). + printf("process %x exited with code %d\n", de.dwProcessId, de.u.ExitProcess.dwExitCode); Don't you want to print this line when de.dwProcessId does equal pi.dwProcessId another general nit: fix the curly brace formating to be consistent. Either under the statement above, on the same line, or under the statement above spaced in. Pick one, stick to it, defend your choice relentlessly until your death. + char EOT[4] = {4, exitCode,0}; // We need the buffer to be big enough to get flushed space between the comma and 0 + length+= 3; + else + length+= 1; needs comments. 1 is for the null, 3 is for the null, and ws. + WCHAR* args = (WCHAR*)LocalAlloc(LMEM_FIXED,length * sizeof(WCHAR)); Space after the comma. + args[length-1] = 0; is this needed? since you are using *cat, shouldn't args always have a trailing null? + printf("failed to connecto to device\n"); mispelling + if (buf[i] == 4) { Weird spacing, and why 4? comment needed. can you just read one byte at a time and putchar it? that will save you a bit of logic. I skipped over OUTPUT_DEBUG_STRING_EVENT, but will review it when the rest is squared away. I think my only major question there was answered on IRC.
Attachment #366817 -
Flags: review?(doug.turner) → review-
Assignee | ||
Comment 7•15 years ago
|
||
Attachment #366817 -
Attachment is obsolete: true
Attachment #367845 -
Flags: review?(doug.turner)
Updated•15 years ago
|
Attachment #367845 -
Flags: review?(doug.turner) → review+
Assignee | ||
Comment 8•15 years ago
|
||
Aki isn't using these tools, so I'm not going to land them
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → WONTFIX
Assignee | ||
Updated•15 years ago
|
Flags: wanted-fennec1.0? → wanted-fennec1.0-
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•