memcpy() and volatile

[ejeffrey]

If you use pack you know that the bytes will be packed in with no padding.  You still need to know the endianness and size, but once you do then you know the bytewise layout.  You can use memcpy on the entire struct as usual to copy it in or out of a buffer to send over the wire, but you can't use normal pointers to individual members because they might be misaligned. 

That is itself a contradiction. If I can use memcpy on the structure then pointers are OK. I think you are however referring to copying the entire structure. I am talking about a structure that spans multiple messages so you must know where the data is by putting it there yourself.

That's why I specifically said you can use memcpy on the entire structure.  Generating a pointer to a sub-element is where you get into trouble.  But you can just use assignment to copy elements in and out.  You actually can use memcpy, which is guaranteed to work with any alignment and works with byte alignment, you just need to carefully generate the pointer to it using the offsetof operator.

Code: [Select]

struct mystruct {
    int16_t a;
    int32_t b[4];
} __attribute__(packed);

void copy_b_to_dest(struct mystruct *foo, int *dest)
{
    char *raw_buf = foo;
    memcpy(dest, raw_buf + offsetof(struct mystruct, b), sizeof(foo->b));
}

Note I haven't compiled this code, it may be buggy and a bit of a silly example.  It's the idea that I'm trying to show how to use memcpy to get stuff out of a packed structure without having alignment problems.

[Andy Watson]

   mpm_to_rpm   0x20002768   const int32_t(static storage at address 0x0.)

This is from the watch window regarding a variable called "mpm_to_rpm" do i read correctly that it is located at address 0 ?

Probably compiler/debugger short-hand for saying that mpm_to_rpm does not require storage space. Its value is completely resolved and can be used directly at compile time - it doesn't need to be stored - at least not in data space.

Defined here at line 26 in motor_control.c:

Code: [Select]

const int16_t mpm_to_rpm               = 38  ; // if changed update min_motor_rpm
it tells the compiler all it needs to know to use the value 38 in the subsequent five references.

[Siwastaja]

you can just use assignment to copy elements in and out.

Which is, btw, what you should be doing most of the time. And not only elements - assigning structs, too.

People overuse stuff like memcpy, memset etc. ignoring the fact that C is high level language.

You can do nice things like copy a struct into another struct of the same type by assignment, clear struct by assigning {0} to it, return a struct from a function call (this is how you can return more stuff than "single variable"(!), pass struct to a function by value, not by pointer...

... and when you do this, many stupid mistakes simply go away, and C becomes a high-level language. These are features people obviously expect from other languages, yet somehow think they don't exist in C. It's weird.

Also don't forget unions. And combinations of unions and structs. You can access the same part of memory in many different ways by using unions. And the access is always correct, and mistakes which are common with memcpy and sizeof become impossible.

When you do that, it pretty much leaves you classical overindexing of array. For which you can develop a habit of always checking before accessing, and using the classic N_ELEMENTS macro I posted above, to further reduce mistakes.

[Simon]

   mpm_to_rpm   0x20002768   const int32_t(static storage at address 0x0.)

This is from the watch window regarding a variable called "mpm_to_rpm" do i read correctly that it is located at address 0 ?

Probably compiler/debugger short-hand for saying that mpm_to_rpm does not require storage space. Its value is completely resolved and can be used directly at compile time - it doesn't need to be stored - at least not in data space.

Defined here at line 26 in motor_control.c:

Code: [Select]

const int16_t mpm_to_rpm               = 38  ; // if changed update min_motor_rpm
it tells the compiler all it needs to know to use the value 38 in the subsequent five references.

Right, so the debugger tries to tell me that the value is effectively optimized (which is what const is for) by telling me that it just put that value at an address it should not be at and with a value that is totally not what I set it too which leaves me wasting hours diagnosing a thing that is not there.

If I take const off again I get the correct value at a RAM address which is what I believe the const "value" above is.

[IanB]

   mpm_to_rpm   0x20002768   const int32_t(static storage at address 0x0.)

This is from the watch window regarding a variable called "mpm_to_rpm" do i read correctly that it is located at address 0 ?

Probably compiler/debugger short-hand for saying that mpm_to_rpm does not require storage space. Its value is completely resolved and can be used directly at compile time - it doesn't need to be stored - at least not in data space.

Defined here at line 26 in motor_control.c:

Code: [Select]

const int16_t mpm_to_rpm               = 38  ; // if changed update min_motor_rpm
it tells the compiler all it needs to know to use the value 38 in the subsequent five references.

Right, so the debugger tries to tell me that the value is effectively optimized (which is what const is for) by telling me that it just put that value at an address it should not be at and with a value that is totally not what I set it too which leaves me wasting hours diagnosing a thing that is not there.

If I take const off again I get the correct value at a RAM address which is what I believe the const "value" above is.

Every debugger I have ever used would tell me that "mpm_to_rpm" is of type int16_t and has a value of 38 decimal.

I do not quite understand where that information you showed came from?

[Simon]

you can just use assignment to copy elements in and out.

Which is, btw, what you should be doing most of the time. And not only elements - assigning structs, too.

People overuse stuff like memcpy, memset etc. ignoring the fact that C is high level language.

You can do nice things like copy a struct into another struct of the same type by assignment, clear struct by assigning {0} to it, return a struct from a function call (this is how you can return more stuff than "single variable"(!), pass struct to a function by value, not by pointer...

... and when you do this, many stupid mistakes simply go away, and C becomes a high-level language. These are features people obviously expect from other languages, yet somehow think they don't exist in C. It's weird.

Also don't forget unions. And combinations of unions and structs. You can access the same part of memory in many different ways by using unions. And the access is always correct, and mistakes which are common with memcpy and sizeof become impossible.

When you do that, it pretty much leaves you classical overindexing of array. For which you can develop a habit of always checking before accessing, and using the classic N_ELEMENTS macro I posted above, to further reduce mistakes.

I only use memcpy where I need to transfer data without casting it. In this program it is used only for handling the data in and out of the CAN system which is why I set up a library of int32 value to simply use throughout the program.

With any structure that I use memcpy on I can't pack it even if I use the offset as you say because the data must arrive at the other end in the same order so that two totally different devices running different programs and are different architectures can communicate.

So take any of my structures, I have 24 bytes to send they are sent in 3 messages of 8 bytes each. each message has a different ID (J1939 / PDO style), this is so that the HMI unit knows what the data is by the message ID and the location in each message of the data. Now if I pack that data and it moves it's game over. If I use the offset to variables in the structure I essentially end up with a custom peice of code for each and every one of my 3 messages and memcpy is really only being used because I am copying into single bytes variables that could have 4 bytes. Same on reception.

The only way around this is an array of variables so that they are all the same type. This is the same problem as the micro controller registers and the chip manufacturers have no problems with the bit fields staying in the right place.

[Simon]

   mpm_to_rpm   0x20002768   const int32_t(static storage at address 0x0.)

This is from the watch window regarding a variable called "mpm_to_rpm" do i read correctly that it is located at address 0 ?

Probably compiler/debugger short-hand for saying that mpm_to_rpm does not require storage space. Its value is completely resolved and can be used directly at compile time - it doesn't need to be stored - at least not in data space.

Defined here at line 26 in motor_control.c:

Code: [Select]

const int16_t mpm_to_rpm               = 38  ; // if changed update min_motor_rpm
it tells the compiler all it needs to know to use the value 38 in the subsequent five references.

Right, so the debugger tries to tell me that the value is effectively optimized (which is what const is for) by telling me that it just put that value at an address it should not be at and with a value that is totally not what I set it too which leaves me wasting hours diagnosing a thing that is not there.

If I take const off again I get the correct value at a RAM address which is what I believe the const "value" above is.

Every debugger I have ever used would tell me that "mpm_to_rpm" is of type int16_t and has a value of 38 decimal.

I do not quite understand where that information you showed came from?

that line is copied and pasted from the microchip studio watch1 window. I'm going to try and convert over to mplabx as I have other issues with studio like it remembering any file ever opened so that if you take a copy of your project and reopen it, it will look at the files in the other copy unless you rename the folder of the previous copy.

[Mechatrommer]

...pass struct to a function by value, not by pointer...

passing struct by pointer has its place for performance reason.. ymmv..

[IanB]

I only use memcpy where I need to transfer data without casting it. In this program it is used only for handling the data in and out of the CAN system which is why I set up a library of int32 value to simply use throughout the program.

With any structure that I use memcpy on I can't pack it even if I use the offset as you say because the data must arrive at the other end in the same order so that two totally different devices running different programs and are different architectures can communicate.

So take any of my structures, I have 24 bytes to send they are sent in 3 messages of 8 bytes each. each message has a different ID (J1939 / PDO style), this is so that the HMI unit knows what the data is by the message ID and the location in each message of the data. Now if I pack that data and it moves it's game over. If I use the offset to variables in the structure I essentially end up with a custom peice of code for each and every one of my 3 messages and memcpy is really only being used because I am copying into single bytes variables that could have 4 bytes. Same on reception.

The only way around this is an array of variables so that they are all the same type. This is the same problem as the micro controller registers and the chip manufacturers have no problems with the bit fields staying in the right place.

I suggest the best way to deal with this is to have a send buffer and a receive buffer, and have pack and unpack routines to put the data in and out of the buffer. The pack and unpack routines would explicitly handle the bit sequence of the data in the payload, take care of endianness as required, and basically would make sure to fill and empty a buffer exactly according to the specifications of the communication protocol.

You are then free to define structs in your code for message content in any natural way, with no worries about packing or alignment, and just have the pack and unpack routines put data in and out of the structs as needed.

This way you will have abstracted the I/O layer into a hardware specific interface, and the rest of your code can be arranged neatly without knowing about the hardware it is running on.

An additional benefit is that you can mock that I/O abstraction layer for testing, and you can compile and test the rest of your code on a platform of your choice like Windows or Unix, and debug the logic using a nice high level IDE with friendly debugging tools.

[Andy Watson]

Right, so the debugger tries to tell me that the value is effectively optimized (which is what const is for) by telling me that it just put that value at an address it should not be at and with a value that is totally not what I set it to.

Not quite. The constant is baked-in to the code at compile time. The value must exist somewhere beacuse a) it is used (5 times), and b) you can create a pointer to it and read back the value ¹. I've just tried writing to a const value, via a pointer (on PC - i686) and, not surprisingly, it seg-faults because the pointer is (I guess) pointing to code memory.

If I take const off again I get the correct value at a RAM address which is what I believe the const "value" above is.

Yes, because you've changed mem_to_rpm into a variable.


1.- I don't know how the value is read-back via a pointer - perhaps the compiler has enough intelligence to be able to resolve a level of indirection?

[tggzzz]

Greybeards remember the truth in an old saying: "cc is half a compiler, the other half is lint". In other words, use lint to detect the fluffs in your code, then remove the fluffs.

Still true, even if lint has been superseded by -Wall and -Wreallyreallyall etc

[DiTBho]

Greybeards remember the truth in an old saying: "cc is half a compiler, the other half is lint". In other words, use lint to detect the fluffs in your code, then remove the fluffs.

Still true, even if lint has been superseded by -Wall and -Wreallyreallyall etc

 

[Perkele]

Still true, even if lint has been superseded by -Wall and -Wreallyreallyall etc

Begin rant.
We're still not there. I'm paying annually four times more for a static analyser than for IDE + compiler suite.
When selecting a static analyser, I evaluated about 12 of them for use with plain C code.
C99 or C11, nothing more, nothing weird, standard code.
Half were completely useless, some of really expensive ones had results worse than splint.
Splint was obsoleted 15 years ago and can't even parse C99 code properly.
End rant.

[tggzzz]

Still true, even if lint has been superseded by -Wall and -Wreallyreallyall etc

Begin rant.
We're still not there. I'm paying annually four times more for a static analyser than for IDE + compiler suite.
When selecting a static analyser, I evaluated about 12 of them for use with plain C code.
C99 or C11, nothing more, nothing weird, standard code.
Half were completely useless, some of really expensive ones had results worse than splint.
Splint was obsoleted 15 years ago and can't even parse C99 code properly.
End rant.

There are many products/processes designed to stabilise sand, e.g. freezing it, injecting it with concrete, driving piles, etc.

But it is better to choose not to build castles on sand.

[Simon]

I only use memcpy where I need to transfer data without casting it. In this program it is used only for handling the data in and out of the CAN system which is why I set up a library of int32 value to simply use throughout the program.

With any structure that I use memcpy on I can't pack it even if I use the offset as you say because the data must arrive at the other end in the same order so that two totally different devices running different programs and are different architectures can communicate.

So take any of my structures, I have 24 bytes to send they are sent in 3 messages of 8 bytes each. each message has a different ID (J1939 / PDO style), this is so that the HMI unit knows what the data is by the message ID and the location in each message of the data. Now if I pack that data and it moves it's game over. If I use the offset to variables in the structure I essentially end up with a custom peice of code for each and every one of my 3 messages and memcpy is really only being used because I am copying into single bytes variables that could have 4 bytes. Same on reception.

The only way around this is an array of variables so that they are all the same type. This is the same problem as the micro controller registers and the chip manufacturers have no problems with the bit fields staying in the right place.

I suggest the best way to deal with this is to have a send buffer and a receive buffer, and have pack and unpack routines to put the data in and out of the buffer. The pack and unpack routines would explicitly handle the bit sequence of the data in the payload, take care of endianness as required, and basically would make sure to fill and empty a buffer exactly according to the specifications of the communication protocol.

You are then free to define structs in your code for message content in any natural way, with no worries about packing or alignment, and just have the pack and unpack routines put data in and out of the structs as needed.

This way you will have abstracted the I/O layer into a hardware specific interface, and the rest of your code can be arranged neatly without knowing about the hardware it is running on.

An additional benefit is that you can mock that I/O abstraction layer for testing, and you can compile and test the rest of your code on a platform of your choice like Windows or Unix, and debug the logic using a nice high level IDE with friendly debugging tools.

You still don't understand, the data being sent could be: int8, uint8, int16, uint16, int32, uint32, bit fields. If you have many variables to send the only way to do this is work out a way where you data to send is stored consecutively so that you can copy it from a start address to an end address. Options that I am aware of are structs and arrays. every message will be potentially  unique in terms of the data. unless you want to create a message specific function for every message it will be a mess.

Again micro controller registers are mapped with structures, if it was not possible to write a structure and have the data appear in memory as written then lots of code will not work! but it does. So all this packed stuff is not essential if you write them properly!

[PlainName]

Options that I am aware of are structs and arrays

Perhaps unions would help with "unless you want to create a message specific function for every message".

http://www.jnkvv.org/PDF/25042020093559244201357.pdf

[IanB]

You still don't understand...

Telling me that I don't understand, when I have been writing software for 40-odd years, is not going to help you.

People have been sending data over the wire for a very long time. It's not a new thing. That's why the OSI model exists.

You may not need all of the layers in every case, but the concept is helpful. Abstraction is key.

And yes, different message types may need to be packed and unpacked differently. This is going to happen no matter how you approach it.

But, as the saying goes, "it's your funeral".

[Perkele]

Still true, even if lint has been superseded by -Wall and -Wreallyreallyall etc

Begin rant.
We're still not there. I'm paying annually four times more for a static analyser than for IDE + compiler suite.
When selecting a static analyser, I evaluated about 12 of them for use with plain C code.
C99 or C11, nothing more, nothing weird, standard code.
Half were completely useless, some of really expensive ones had results worse than splint.
Splint was obsoleted 15 years ago and can't even parse C99 code properly.
End rant.

There are many products/processes designed to stabilise sand, e.g. freezing it, injecting it with concrete, driving piles, etc.

But it is better to choose not to build castles on sand.

Time machines are not available to general population, regardless of project's budget.
Device in question was built a decade ago, and there's about 100k lines of code to maintain.

[SiliconWizard]

Greybeards remember the truth in an old saying: "cc is half a compiler, the other half is lint". In other words, use lint to detect the fluffs in your code, then remove the fluffs.

Still true, even if lint has been superseded by -Wall and -Wreallyreallyall etc

Yes, well, there are "many" static analyzers way better than lint ever was now, some "free", some very expensive. They'll "beat" any compiler's analysis, even Clang, which has already a much better static analyzer than GCC.
So definitely worth using them. Compilers' warnings are better than nothing, but definitely pretty basic in terms of analysis.

[tggzzz]

Still true, even if lint has been superseded by -Wall and -Wreallyreallyall etc

Begin rant.
We're still not there. I'm paying annually four times more for a static analyser than for IDE + compiler suite.
When selecting a static analyser, I evaluated about 12 of them for use with plain C code.
C99 or C11, nothing more, nothing weird, standard code.
Half were completely useless, some of really expensive ones had results worse than splint.
Splint was obsoleted 15 years ago and can't even parse C99 code properly.
End rant.

There are many products/processes designed to stabilise sand, e.g. freezing it, injecting it with concrete, driving piles, etc.

But it is better to choose not to build castles on sand.

Time machines are not available to general population, regardless of project's budget.
Device in question was built a decade ago, and there's about 100k lines of code to maintain.

Am I hallucinating, or does the Linux kernel still mandate C89?

Some long-lived projects are in the position of having to continue using an ~35yo language.

At least they avoid running into the trap of having a valid program that must never finish compiling - because compiling the program causes the compiler to emit the sequence of prime numbers as a side effect!

[SiliconWizard]

Am I hallucinating, or does the Linux kernel still mandate C89?

I haven't checked that, but it may be possible... if just for the fact that they still need to support platforms for which there may not be any C compiler supporting anything other that C89. (Which, if you read DiTBho's posts, wouldn't be so surprising.)

That would be pretty unfortunate indeed, as C99 has brought many nice features, and C11 even more so.

[golden_labels]

What Linux uses is currently GNU C11 (-std=gnu11 option to gcc).

[ejeffrey]

So take any of my structures, I have 24 bytes to send they are sent in 3 messages of 8 bytes each. each message has a different ID (J1939 / PDO style), this is so that the HMI unit knows what the data is by the message ID and the location in each message of the data. Now if I pack that data and it moves it's game over. If I use the offset to variables in the structure I essentially end up with a custom peice of code for each and every one of my 3 messages and memcpy is really only being used because I am copying into single bytes variables that could have 4 bytes. Same on reception.

This is incomprehensible without a code example.  Also three individual serialization routines for three different 24 byte messages is 15 minutes of work and you have been ranting about this for weeks, if not longer.  If that's really what this is about then that seems totally out of proportion.

[Simon]

So take any of my structures, I have 24 bytes to send they are sent in 3 messages of 8 bytes each. each message has a different ID (J1939 / PDO style), this is so that the HMI unit knows what the data is by the message ID and the location in each message of the data. Now if I pack that data and it moves it's game over. If I use the offset to variables in the structure I essentially end up with a custom peice of code for each and every one of my 3 messages and memcpy is really only being used because I am copying into single bytes variables that could have 4 bytes. Same on reception.

This is incomprehensible without a code example.  Also three individual serialization routines for three different 24 byte messages is 15 minutes of work and you have been ranting about this for weeks, if not longer.  If that's really what this is about then that seems totally out of proportion.

It's the principle! remember, you do it today because although not a scalable solution it's a small problem. Then in the future the problem scales, the solution cannot and you have to reinvent the wheel. Tell me, how do J1939 auto ECU's manage this problem? they have several hundred messages of no same format, you are telling me that someone wrote a message handler for each one? I may be inexperienced but I not stupid!

My point about the mapping of hardware registers is constantly being ignored. Fact! hardware registers are mapped into structs. No one has problems with the data being in the location it is supposed to be do they? They don't use pack.

You can either offer a proper answer and stop telling me I am ranting for weeks or continue to belittle me. I'll tell you something, every time I have an issue and come on here I get the same types of responses from  experienced people, and every time the answerer are different from last time. Everyone has an opinion and they treat it as fact. The language has been going for years and has been used in many situations, but no one seems to realize that the model of their little part of life with C is not the same as the one someone else may be in. I am sick of conflicting answers and people ranting about something else,

[IanB]

It's the principle! remember, you do it today because although not a scalable solution it's a small problem. Then in the future the problem scales, the solution cannot and you have to reinvent the wheel. Tell me, how do J1939 auto ECU's manage this problem? they have several hundred messages of no same format, you are telling me that someone wrote a message handler for each one?

In a sense, maybe yes.

In at the hardware level, on a message bus, each device will only pick out and process messages it is interested in. It will look at the PGN and SPN to decide this, and if it doesn't care it will ignore the message. So it only locally has to handle a few kinds of message.

On the other hand, if someone is writing a debugger or bus sniffer that needs to decode and display all kinds of messages, then it will indeed have to handle dozens or hundreds of message types. But it won't do this with huge amounts of code, it will use a database where it looks up the message type and gets the decode information from the database, so it can automate the process. It will use this database to figure out how to unpack and decode the contents of each message.