This removes a lot of boilerplate, which was needed to execute monitor operations. Previously one
needed do declare a separate class for each operation which would manually capture all needed
arguments, which was very verbose. In addition to less code, I believe this also makes the code
more readable, since now the implementation of the operation can be physically closer to the code
that invokes it.
Details
Diff Detail
Event Timeline
source/Plugins/Process/Linux/NativeProcessLinux.cpp | ||
---|---|---|
3171 | Why not make this void * to begin with? | |
3178 | Does it make sense to have PtraceWrapper return Error? To be consistent with some changes you made above. | |
3190 | void *? | |
3728 | Nit: extra space. | |
source/Plugins/Process/Linux/NativeProcessLinux.h | ||
142 | Nit: extra space. |
source/Plugins/Process/Linux/NativeProcessLinux.cpp | ||
---|---|---|
3171 | Whoops, didn't see the assignment right below. Please ignore this. |
source/Plugins/Process/Linux/NativeProcessLinux.cpp | ||
---|---|---|
3171 | This could be made simpler, but I did not want to do it here, as it is just copy-pasted. | |
3178 | I've thought about that as well and I think it is a good idea. I propose to do it in a followup patch. I'll need to check if putting the current return value in a by-ref parameter will not make other things more awkward. | |
3728 | Done. | |
source/Plugins/Process/Linux/NativeProcessLinux.h | ||
142 | On which side? :) |
Looks good, but please check it compiles on all supported architecture (x86_64, arm, aarch64, mips/mips64) because some of the code only used on one of these.
source/Plugins/Process/Linux/NativeProcessLinux.cpp | ||
---|---|---|
253 | (nit) You don't need this. ("return Error();") | |
259 | (nit): You can do a "return Error();" here | |
713 | (nit): I think "m_operation->operator()();" is cleaner but it is up to you |
source/Plugins/Process/Linux/NativeProcessLinux.cpp | ||
---|---|---|
259 | Sorry, I missed that call |
LGTM.
source/Plugins/Process/Linux/NativeProcessLinux.cpp | ||
---|---|---|
3178 | I think it might be better to pass a pointer instead, for consistency with the other arguments, and makes it optional. |
Nit: extra space.