various fixups for atomic operation APIs

[andrewboie]

• fix return value type of atomic_cas() being bool in some cases, int in others
• add atomic_ptr_t typedef and atomic_ptr_cas(), atomic_ptr_set(), atomic_ptr_get(), atomic_ptr_clear() APIs
• fix a type safety issue in shell

Fixes: #22887

[zephyrbot]

All checks are passing now.

checkpatch (informational only, not a failure)

-:82: WARNING:NEW_TYPEDEFS: do not add new typedefs
#82: FILE: include/sys/atomic.h:24:
+typedef void *atomic_ptr_t;

-:204: WARNING:SPACING: space prohibited between function name and open parenthesis '('
#204: FILE: include/sys/atomic.h:293:
+	defined (CONFIG_ATOMIC_OPERATIONS_C)

- total: 0 errors, 2 warnings, 392 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

[nordic-krch]

@andrewboie given your comment (#22887 (comment)) should then atomic_t become uintptr_t or slong_t? What's pros and cons? They are same size on 32bit and 64bit so maybe it does not matter much.

[nordic-krch]

I think you can add "fixes #22887" to the description.

[andrewboie]

@andrewboie given your comment (#22887 (comment)) should then atomic_t become uintptr_t or slong_t? What's pros and cons? They are same size on 32bit and 64bit so maybe it does not matter much.

atomic_t is currently a signed integer. using uintptr_t would convert it to an unsigned value, which I do not want to do in this PR.

[joerchan]

@andrewboie given your comment (#22887 (comment)) should then atomic_t become uintptr_t or slong_t? What's pros and cons? They are same size on 32bit and 64bit so maybe it does not matter much.

atomic_t is currently a signed integer. using uintptr_t would convert it to an unsigned value, which I do not want to do in this PR.

Is a long guaranteed to be the same size as a pointer? AFAIK it is only guaranteed to be 32 bits or more.

[andrewboie]

@andrewboie given your comment (#22887 (comment)) should then atomic_t become uintptr_t or slong_t? What's pros and cons? They are same size on 32bit and 64bit so maybe it does not matter much.

atomic_t is currently a signed integer. using uintptr_t would convert it to an unsigned value, which I do not want to do in this PR.

Is a long guaranteed to be the same size as a pointer? AFAIK it is only guaranteed to be 32 bits or more.

I used one of zephyr's derived types in zephyr/types.h. I did not typedef atomic_t to long.

slong_t / ulong_t are defined as being able to hold a pointer. They are that header's version of uintptr_t / intptr_t

[andyross]

We can't make atomic_t be an architecture-dependent size. Users need to be able to store known quantities in it. And on a practical level: 64 bit platforms need to be able to make atomic updates to 32 bit ints too.

[andrewboie]

We can't make atomic_t be an architecture-dependent size. Users need to be able to store known quantities in it. And on a practical level: 64 bit platforms need to be able to make atomic updates to 32 bit ints too.

What do you suggest we do?
I'm loath to introduce a parallel set of 64-bit wide atomic APIs, signed vs unsigned, etc

[andyross]

I don't see that we have much choice. Apps want a consistent data type for atomic operations (e.g lock counts) that isn't architecture-dependent. But they also want to be able to store a pointer/address in them. I think having a separate "atomic_addr_t" (which on 32 bit systems would just be implemented as a atomic_t) is reasonable enough.

And I take no position on signed/unsigned. From my perspective it's perfectly OK to have it be any integer type and let the applications cast as needed. The feature being provided is about the memory underneath the type, not the bytes themselves.

[joerchan]

What about the original suggestion:

Create atomic_ptr_t type and set of atomic_ptr_cas(), atomic_ptr_set(), atomic_ptr_get() and atomic_ptr_clear() functions.

[andrewboie]

What about the original suggestion:

Create atomic_ptr_t type and set of atomic_ptr_cas(), atomic_ptr_set(), atomic_ptr_get() and atomic_ptr_clear() functions.

Yeah, you guys have convinced me ... updated PR in a bit

[andrewboie]

Updated PR ... adds atomic_ptr_* APIs for cas, get, set, clear.