New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PATCH] Return through pointer arguments #4

Closed
malsyned opened this Issue Aug 12, 2012 · 9 comments

Comments

Projects
None yet
4 participants
@malsyned

malsyned commented Aug 12, 2012

As discussed at http://throwtheswitch.org/white-papers/cmock-pointers-and-structs.html, CMock doesn't provide a way for mocks to modify the data pointed to by pointer arguments. This makes it impossible to mock functions that return data through pointers. That article discusses workarounds, but they all assume that you have control over the API of the function in question. Sometimes, you don't.

I've developed a patch to add this ability to CMock: [EDIT: Link removed due to bugs. In a later comment I posted a link to a fixed version with additional features]

The patch adds a plugin, return_thru_ptr. When this plugin is enabled, additional macros are generated for each function's pointer arguments which allow you to specify data to be copied onto the target of that pointer by the mock. For example, for a function with the prototype void ptr_ret_int(int *r), the following macros are created:

void ptr_ret_int_ReturnThruPtr_r(int *r)
void ptr_ret_int_ReturnArrayThruPtr_r(int *r, int len)
void ptr_ret_int_ReturnMemThruPtr_r(int *r, int size)

These functions can be called after one of the Expect family of calls to give the most recently defined expectation a value to return through a pointer argument.

ReturnThruPtr arranges for a single value to be copied onto the target of the pointer argument.
ReturnArrayThruPtr arranges for an array of values, with a length specified by len, to be copied.
ReturnMemThruPtr arranges for a memory buffer, with a size specified in bytes, to be copied.

Here is an example of a test that exercises ReturnThruPtr for a function void ptr_ret_ints(int *r, int *s):

{
    int r, s = 0x0880AA55;
    int r_res = 4;
    int s_res = 6;

    Mock_ptr_ret_Init();

    ptr_ret_ints_Expect(&r, &s);
    ptr_ret_ints_ReturnThruPtr_r(&r_res);
    ptr_ret_ints_ReturnThruPtr_s(&s_res);

    ptr_ret_ints(&r, &s);

    Mock_ptr_ret_Verify();
    TEST_ASSERT_EQUAL(4, r);
    TEST_ASSERT_EQUAL(6, s);
}

Usage Notes

The expectation object created by these calls only stores a pointer to the memory to be copied, so it's important that the memory, in this case r_res and s_res, not be modified between the calls to ReturnThruPtr and the call to the mocked function.

ReturnThruPtr functions must be called after the Expect calls they modify. A ReturnThruPtr call before any Expect calls will result in undefined behavior.

return_thru_ptr doesn't work with the ignore plugin. It only properly returns through pointer arguments when the expectation was created with an Expect call.

Implementation Notes

The CMock plugin architecture isn't quite flexible enough to support implementing this feature entirely within the plugin class file. Since there's no structured way to add additional constructor code to expectations, I had to hack cm_utils.code_add_argument_loader to detect the presence of the return_thru_ptr plugin and add initialization of the ReturnThruPtr_..._Used fields, using the same technique that code_add_argument_loader already used to hack in support for the array plugin.

While incomplete types won't prevent mocks from being generated or compiled with return_thru_ptr enabled, they will present an obstacle for anyone wanting to actually use the plugin to pass data through an incompletely typed argument. This can be worked around with ReturnMemThruPtr but requires hard-coding the size of the abstract type, which is very dangerous and brittle. A better solution is to use preprocessor magic to make sure that the type is complete when compiling the test harness.

@malsyned

This comment has been minimized.

Show comment
Hide comment
@malsyned

malsyned Aug 13, 2012

As I'm trying to make use of this feature on a real project, I'm seeing that its usefulness is limited because the expect plugin still checks the passed-in pointer, and you hardly ever actually care about that. What one really wants to say is something like this:
Expect a call to my_func. Expect arg_1 to be 5. Expect arg_2 to be an array. Ignore the passed-in value for the pointer arg_3, but copy this data onto its target.
I'm working on a per-argument ignore option, but cmock's plugin system isn't really built to make that sort of thing easy. I've got an idea for how to fit it in, but I'm open to guidance to do it in a way that is upstream-friendly.

malsyned commented Aug 13, 2012

As I'm trying to make use of this feature on a real project, I'm seeing that its usefulness is limited because the expect plugin still checks the passed-in pointer, and you hardly ever actually care about that. What one really wants to say is something like this:
Expect a call to my_func. Expect arg_1 to be 5. Expect arg_2 to be an array. Ignore the passed-in value for the pointer arg_3, but copy this data onto its target.
I'm working on a per-argument ignore option, but cmock's plugin system isn't really built to make that sort of thing easy. I've got an idea for how to fit it in, but I'm open to guidance to do it in a way that is upstream-friendly.

@malsyned

This comment has been minimized.

Show comment
Hide comment
@malsyned

malsyned Aug 13, 2012

I just found a bug in my implementation. I'm working on a fix now and I'll post an updated patch when I'm done.

I have tests for this plugin, but they aren't included in the patch because the CMock test suite wouldn't run for me and I want to integrate them there.

malsyned commented Aug 13, 2012

I just found a bug in my implementation. I'm working on a fix now and I'll post an updated patch when I'm done.

I have tests for this plugin, but they aren't included in the patch because the CMock test suite wouldn't run for me and I want to integrate them there.

@malsyned

This comment has been minimized.

Show comment
Hide comment
@malsyned

malsyned Aug 14, 2012

Fixed version of the patch, with additional features, posted here: [EDIT: Link removed. The patch is superseded by this pull request: https://github.com/ThrowTheSwitch/CMock/issues/9]

In addition to fixing the multiple-expectation issue from my previous patch, this one adds an additional plugin called ignore_arg. When this plugin is enabled, a macro is generated for every function argument which allows you to specify that the value of that argument should be ignored by the most recently defined expectation.

For example, for a function with the prototype void my_func(int arg1, int arg2), the following macros are created:

void my_func_IgnoreArg_arg1(void)
void my_func_IgnoreArg_arg2(void)

These functions can be called after one of the Expect family of calls to instruct the most recently defined expectation to ignore the specified argument.

If both ignore_arg and return_thru_ptr are loaded, then a ReturnThruPtr call automatically implies IgnoreArg for that argument.

malsyned commented Aug 14, 2012

Fixed version of the patch, with additional features, posted here: [EDIT: Link removed. The patch is superseded by this pull request: https://github.com/ThrowTheSwitch/CMock/issues/9]

In addition to fixing the multiple-expectation issue from my previous patch, this one adds an additional plugin called ignore_arg. When this plugin is enabled, a macro is generated for every function argument which allows you to specify that the value of that argument should be ignored by the most recently defined expectation.

For example, for a function with the prototype void my_func(int arg1, int arg2), the following macros are created:

void my_func_IgnoreArg_arg1(void)
void my_func_IgnoreArg_arg2(void)

These functions can be called after one of the Expect family of calls to instruct the most recently defined expectation to ignore the specified argument.

If both ignore_arg and return_thru_ptr are loaded, then a ReturnThruPtr call automatically implies IgnoreArg for that argument.

@malsyned

This comment has been minimized.

Show comment
Hide comment
@malsyned

malsyned Aug 14, 2012

After more use on real projects, I've changed my copy to remove the automatic IgnoreArg feature from ReturnThruPtr. Sometimes, you want to both check what is passed in and return something else. Now that's possible. I haven't posted a patch for this change. It's easy enough to make in the return_thru_ptr source.

malsyned commented Aug 14, 2012

After more use on real projects, I've changed my copy to remove the automatic IgnoreArg feature from ReturnThruPtr. Sometimes, you want to both check what is passed in and return something else. Now that's possible. I haven't posted a patch for this change. It's easy enough to make in the return_thru_ptr source.

@barneywilliams

This comment has been minimized.

Show comment
Hide comment
@barneywilliams

barneywilliams Sep 6, 2012

Member

#malsyned,

Thanks for the investigation and patches. We will look at folding this in after some validation.

Sorry for the delay!

Greg

Member

barneywilliams commented Sep 6, 2012

#malsyned,

Thanks for the investigation and patches. We will look at folding this in after some validation.

Sorry for the delay!

Greg

@mvandervoord

This comment has been minimized.

Show comment
Hide comment
@mvandervoord

mvandervoord Mar 7, 2014

Member

this is merged at this point.

Member

mvandervoord commented Mar 7, 2014

this is merged at this point.

@andreasosio

This comment has been minimized.

Show comment
Hide comment
@andreasosio

andreasosio Oct 20, 2016

Not sure why this has not been raised before. If the purpose is to allow for the mocking of functions that return certain values and do that through pointers, why don't the macros let you specify the values directly, instead of requiring that they be passed via pointers? In other words, why couldn't the example be:

{
int r, s = 0x0880AA55;

Mock_ptr_ret_Init();

ptr_ret_ints_Expect(&r, &s);
ptr_ret_ints_ReturnThruPtr_r(4); // who cares about the r_res variables and their addresses? We just want 4 to be stored in r...
ptr_ret_ints_ReturnThruPtr_s(6);

ptr_ret_ints(&r, &s);

Mock_ptr_ret_Verify();
TEST_ASSERT_EQUAL(4, r);
TEST_ASSERT_EQUAL(6, s);

}

Having to pass values through a pointer seems unnecessary (from a user point of view, I mean) and makes it awkward and dangerous to create reusable functions that setup mocks for different tests.

andreasosio commented Oct 20, 2016

Not sure why this has not been raised before. If the purpose is to allow for the mocking of functions that return certain values and do that through pointers, why don't the macros let you specify the values directly, instead of requiring that they be passed via pointers? In other words, why couldn't the example be:

{
int r, s = 0x0880AA55;

Mock_ptr_ret_Init();

ptr_ret_ints_Expect(&r, &s);
ptr_ret_ints_ReturnThruPtr_r(4); // who cares about the r_res variables and their addresses? We just want 4 to be stored in r...
ptr_ret_ints_ReturnThruPtr_s(6);

ptr_ret_ints(&r, &s);

Mock_ptr_ret_Verify();
TEST_ASSERT_EQUAL(4, r);
TEST_ASSERT_EQUAL(6, s);

}

Having to pass values through a pointer seems unnecessary (from a user point of view, I mean) and makes it awkward and dangerous to create reusable functions that setup mocks for different tests.

@mvandervoord

This comment has been minimized.

Show comment
Hide comment
@mvandervoord

mvandervoord Oct 20, 2016

Member

Thanks for the good question.

There is actually a good reason. The case you have cited above, while it happens, is a rare one. Most often when a pointer is returned, it's because the thing that is being pointed to is something too large or cumbersome to return directly. It's not often that people return a pointer to a single integer... it's much more common that they return a pointer to an array or a struct or even an array of structs. In all of those cases, it's much more convenient in C to break this into two steps.

If C were a more flexible language, we could accept both ways... Unfortunately it's not really good at overloading like this.

I hope this helps!

Member

mvandervoord commented Oct 20, 2016

Thanks for the good question.

There is actually a good reason. The case you have cited above, while it happens, is a rare one. Most often when a pointer is returned, it's because the thing that is being pointed to is something too large or cumbersome to return directly. It's not often that people return a pointer to a single integer... it's much more common that they return a pointer to an array or a struct or even an array of structs. In all of those cases, it's much more convenient in C to break this into two steps.

If C were a more flexible language, we could accept both ways... Unfortunately it's not really good at overloading like this.

I hope this helps!

@andreasosio

This comment has been minimized.

Show comment
Hide comment
@andreasosio

andreasosio Oct 20, 2016

Thanks mvandervoord, you're right that both ways would be useful, but I understand your point.
Hopefully we can devise an effective workaround for our purposes.

andreasosio commented Oct 20, 2016

Thanks mvandervoord, you're right that both ways would be useful, but I understand your point.
Hopefully we can devise an effective workaround for our purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment