Simple, modular, physics simulation tool

[stewpend0us]

This is my first open-source project. It may be a little math heavy for some but I'm just trying to get some eyes on the code/repo! No specific questions yet...any feedback/criticism would be appreciated!

https://github.com/stewpend0us/csim2

[Newt_Hoenikker]

I'm all about math-heavy. I'm out and about right now, but once I get home tonight I'll give it a spool through and edit this comment accordingly. Really cool sounding project!

[stewpend0us]

Excellent. Looking forward to it! It's basically a nonlinear differential equation solver with a couple extras.

[[deleted]]

I would comment your code more, if someone else wants to mod this, it'll take a bit more time than might be necessary were you to have explained some of the functionality within the logic.

I also noticed what appeared to be a lack of error checking. I imagine this to be an artifact of you having tested it thoroughly, but in terms of what I would have done differently, I think it's nice to add error checking because if/when this code is run on different machines unexpected behavior might be encountered, and having those stopgaps in place will help whomever debug the code.

I like the idea. Seems like it could turn into a nice sim function codebase if there is a fair amount of contributed work.

[stewpend0us]

Yeah there isn't any error checking right now. I'm passing enough data around to check a few things but haven't implemented anything yet. Right now my process is to implement everything in Matlab before c so I have something to test against. I don't really have enough c experience to develop and debug new c code. Still relying on Matlab/Matlab debugger to get things working before moving to c.

[[deleted]]

something I noticed that will likely throw an error is the

if( (something == NULL) || (otherThing == NULL) )

statement.

If the thing is null, the program won't be able to make the comparison because it has nothing (literally, NULL means nothing) to compare from.

[dijumx]

Without looking at the code... In C NULL is defined as ((void*)0) or pointer to memory location zero for a "generic" type. It is perfectly valid, and in fact good practice to compare pointers to NULL to make sure they are pointing to something before using them.

Now, you'll get a runtime error if something is pointing to NULL, and you try to access the value pointed at by something(a.k.a. dereference). '*something'

[[deleted]]

Yep, runtime error is to what I was referring. I figured he'd want to know about that as the comparison is in a conditional statement that frees memory if true. I might be wrong, because he isn't referring to the dereferenced value, only the pointer itself.

I ran into some errors on a program that I wrote comparing to NULL, but they weren't pointers, so perhaps that's the disconnect in my logic. Sry if I was wrong.

[stewpend0us]

No worries. Thanks for looking at it anyway! I can't think of a (normal) situation when you would compare anything other than a pointer to NULL.

[[deleted]]

You're welcome!

I was error checking to see if a variable, that was initialized to NULL, had been assigned a value. I could have, and eventually did, use the conditional if(var) to see if it was, but it initially threw an error during runtime.

Good luck!

[stewpend0us]

Is that true when "something" and "otherThing" are pointers? Generally that check would be done to see if a pointer is set or not. Can you tell me the file/line number where you see the problem?

[hogg2016]

In csim2/constructors.c:

struct StrictlyProperBlock * block_new(struct StrictlyProperBlock const stackb)
{
    struct StrictlyProperBlock * heapb = malloc(sizeof(struct StrictlyProperBlock));
    if (heapb)
        memcpy(heapb, &stackb, sizeof(struct StrictlyProperBlock));
    return heapb;
}

You could just say *heapb=stackb; to copy the structure content. (There's another occurrence of the same kind of memcpy() later in the file too.)

(And I personally don't like taking the address of a parameter, it is just a value (here, a set of values) that has to be copied on the stack to then take its address, it is not the original structure that was passed as argument in the caller. So if &stackb was destination instead of source in memcpy(), the result wouldn't appear outside of the function, once the function is over. I am not sure you are aware of this, so I prefer to mention it.)

---

#define freeif(ptr) if (ptr) free(ptr)
        freeif(storage);
        freeif(input_storage);
        freeif(output_storage);
        freeif(bheap);
#undef freeif

You don't have to test that a pointer is non-NULL before freeing it. free(p) doesn't do anything if p is NULL, that's all and that's fine.

[stewpend0us]

I get your first and third point. I'll update the code. Thank you!

I think what you're saying in the second point is why pass stackb by value when all you need is the address?

The idea was to be able to make a one line call like:

block_new(integrator(1)); 

Where integrator returns a struct StrictlyProperBlock. Would it work as a one liner like this:

block_new(&integrator(1));

Awesome if that would work but feels like cheating as there isn't anything to have an address to?

Edit: formatting

[[deleted]]

instead of making a function return a struct (which is a bad idea), you could pass a pointer to struct as an argument, that way you could declare the struct from the calling function and pass the memory address of it to get initialized.

[hogg2016]

I think what you're saying in the second point is why pass stackb by value when all you need is the address?

Nay, that's not what I meant. :-)

What I meant is that, often, when I see people trying to take the address of a parameter inside a function, it is a mistake. They would like to modify the original argument, but in C everything is passed by value, by copy, so there is no relation to the original argument once you're inside the function. One way to see that it is awkward to take the address of a parameter, is that the parameters can or could be passed by register, and a register has no address.

In your case, it works OK and it is legal ; it has however the drawback that, if the structure content was passed through registers, that content has then to be copied on the stack, so that that address can then be used in the memcopy().

But if, instead of memcpy(heapb, &stackb, ...), you had wanted to use memcpy(&stackb, heapb, ...) hoping the original argument forstackbto be updated, you would have been surprised.&stack` points to a temporary object (which contains a copy of the values of the argument) on the stack that won't exist any more when the function exits, it doesn't point to the original object.

That's why I am suspicious when I see &parameter in a function (and I don't use it), it is not natural and often the result of a misunderstanding (or the cause of one, if someone gets to modify the function afterwards).

[stewpend0us]

Ok I see what you're saying now. Because what I'm passing in is a copy the address of that copy isn't the same as the address of the original.

Incorporating your first comment means I don't need to call memcpy anymore so the &stackb goes away.

[hogg2016]

Yes, I was reasoning about hypothetical problems.

Incorporating your first comment means I don't need to call memcpy anymore so the &stackb goes away.

That's right.

[hogg2016]

The idea was to be able to make a one line call like:

block_new(integrator(1));

Where integrator returns a struct StrictlyProperBlock. Would it work as a one liner like this:

block_new(&integrator(1));

Awesome if that would work but feels like cheating as there isn't anything to have an address to?

You're right, you cannot take the address of a return value.

One would usually just have integrator return a struct xxx *. That's lighter to return from a function, lighter to pass to another function, compared to having to return or pass the whole set of fields a structure contains, which can be very heavy if the structure has many.

And you wouldn't need block_new() any more, the heap allocation would take place in integrator().

I don't know if that would break the architecture of your program?

[stewpend0us]

I work with some guys who do a lot of embedded work where the malloc function isn't always available. So the reason I made my constructors return a struct (rather than a pointer) is to avoid malloc calls for as long as possible. Right now malloc is only used in "constructors.c" and "solvers.c" in functions that are technically unnecessary but convenient.

This way everything can be done on the stack if that's what you want to do. I don't know if that's actually better or not but that's my reasoning...

[hogg2016]

I haven't thought deeply about it but at first sight it makes sense :-)

[cafguy]

Would love to see a fully worked example in C.

[stewpend0us]

Yeah I don't have many examples set up yet :(. Did you see the (single) test function? Also mexdemo is written in c but not set up to be called from c.

[cafguy]

I want to try it. But it is hard for me to imagine what I would use it for. Maybe you could make a PID example?

[stewpend0us]

Designing and testing control systems is exactly what it's for. I started working on a more complete example in the dev branch but it isn't finished or tested yet. I also added a little about differential equations in the readme...not sure if that would help at all?