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.