Subject: |
Re: [Merge] lp:~epics-core/epics-base/epicsR3.15-atomics into lp:epics-base |
From: |
Jeff Hill <[email protected]> |
To: |
Jeff Hill <[email protected]> |
Date: |
Fri, 19 Aug 2011 22:15:23 -0000 |
Responding to Michael's comments:
> 1) How about add and subtract? For instructions sets which don't support this directly
> it can be implemented in terms of compare+swap.*
That interface would probably be best implemented with signed, and not unsigned, integer types. My current position (subject to change :-) is that I don't have a need for this but if you do then any such features should not delay the initial merge unless you do have immediate needs. I have a large amounts of code to be merged, and all of it is potentially blocked while this merge is pending. Also, there is daily code maintenance merging in changes from the main trunk while this merge is pending.
> 2) I would suggest not making the locked functions part of the public API. I don't see one
> piece of code needing to use both epicsLockedIncrSizeT and epicsAtomicIncrSizeT. Perhaps
> instead have a macro to force a fallback to the default implementation?
The original reasons for the epicsLockedXxxx public interface was so that we _could_ have macros forwarding the epicsAtomixXxxx to epicsLockedXxxx. That requires a visible function prototype somewhere for the epicsLockedXxxx interface. However, it turns out that independently I recently eliminated the epicsLockedXxxx interface (see revision 12248) because I decided that it was simpler and more efficient to just implement the locked version in OS specific code. So I didn't like the epicsLockedXxxx functions polluting the public interface either, and they have now been eliminated.
> 3) As a thought, how about signed instead of unsigned integers? This
> would differentiate 'int' from 'size_t' operations even on 32-bit machines.
I implemented the unsigned set/get/compare-and-swap interface for compare-and-swap applications where one is manipulating enumerated states.
Personally, I very rarely use signed integers in my code because I don't like the overhead of sanity checking both sides of the range. So that hopefully explains how I ended up with size_t and unsigned which are admittedly the same on 32 bits.
So another option would be to implement a "unsigned short" interface which would be arguably distinct from size_t on _all_ architectures, would handle arguably a sufficient range of compare-and-swap enumerated flags, and would involve less storage penalty. You would loose the negative range for compare-and-swap enumerated flags compared to "signed short" but does anyone really need a negative range for compare-and-swap enumerated flags? From my perspective adding that negative range only opens up opportunities for bugs resulting from a negative out of range array index. Of course its a bad idea to mix integers and unsigned integers together in arithmetic expressions so if you start with unsigned enumerated flags you have to stay consistent. Admittedly, the K&R C guys made the maybe bad choice to make C enum type be always negative range capable, but that's unchangeable and this _is_ therefore an argument in favor of using instead "signed short" (there I go arguing against my!
self again :-). However note that enum is the same size as int so there would have to be a range check before converting enum to signed or unsigned short (that's an argument for your original suggestion of an int). So let me know what your consensus is and I will implement it - one of {int, unsigned short, or signed short} for the set/get/compare-and-swap integer interface. I will vote for "unsigned short" based on my needs (I can use "static const unsigned short flagXxx = nnn" instead of enum for my flag values).
I do agree that if we implement an add and subtract interfaces then that _should_ use signed integers.
--
https://code.launchpad.net/~epics-core/epics-base/epicsR3.15-atomics/+merge/70642
Your team EPICS Core Developers is requested to review the proposed merge of lp:~epics-core/epics-base/epicsR3.15-atomics into lp:epics-base.
- References:
- [Merge] lp:~epics-core/epics-base/epicsR3.15-atomics into lp:epics-base Jeff Hill
- Navigate by Date:
- Prev:
Re: [Merge] lp:~epics-core/epics-base/epicsR3.15-atomics into lp:epics-base Jeff Hill
- Next:
[Merge] lp:~anj/epics-base/epicsEvent-api into lp:epics-base noreply
- Index:
2002
2003
2004
2005
2006
2007
2008
2009
2010
<2011>
2012
2013
2014
2015
2016
2017
2018
2019
2020
2021
2022
2023
2024
- Navigate by Thread:
- Prev:
Re: [Merge] lp:~epics-core/epics-base/epicsR3.15-atomics into lp:epics-base Jeff Hill
- Next:
Re: [Merge] lp:~epics-core/epics-base/epicsR3.15-atomics into lp:epics-base Jeff Hill
- Index:
2002
2003
2004
2005
2006
2007
2008
2009
2010
<2011>
2012
2013
2014
2015
2016
2017
2018
2019
2020
2021
2022
2023
2024
|