search

nasa / cFE

The Core Flight System (cFS) Core Flight Executive (cFE)
Apache License 2.0
408 stars 200 forks source link

Correct const in generic memory pool APIs #1348

Open skliper opened 3 years ago

skliper commented 3 years ago

Is your feature request related to a problem? Please describe. CFE_ES_GenPoolRecord_t pointer should be const: https://github.com/nasa/cFE/blob/e80aae94e0f56b868657daba965c590766a4dc57/modules/es/fsw/src/cfe_es_generic_pool.h#L234 https://github.com/nasa/cFE/blob/e80aae94e0f56b868657daba965c590766a4dc57/modules/es/fsw/src/cfe_es_generic_pool.h#L247 https://github.com/nasa/cFE/blob/e80aae94e0f56b868657daba965c590766a4dc57/modules/es/fsw/src/cfe_es_generic_pool.h#L261

Describe the solution you'd like Make const

Describe alternatives you've considered None

Additional context Code review

Requester Info Jacob Hageman - NASA/GSFC

jphickey commented 3 years ago

I suppose we could do this, but note these are internal APIs that deal with internal data structures that are really never const. We don't try to enforce const-ness with the other resource types, either. So this is one case where I'm not sure the const qualifier really provides any benefit.

skliper commented 3 years ago

... really never const

I don't follow. Either it's const or not wrt the API. It's helpful in terms of maintenance/documentation to be correct, still sounds like a valuable enhancement to me even though it's internal.

thnkslprpt commented 1 year ago

If there's still appetite for making these parameters const (where possible), I checked and 3 of the functions in cfe_es_generic_pool.h have CFE_ES_GenPoolRecord_t inputs that can be qualified as const in the declaration: https://github.com/nasa/cFE/blob/7a220ae809555cad86fb98d823ec77528a2fb125/modules/es/fsw/src/cfe_es_generic_pool.c#L53 https://github.com/nasa/cFE/blob/7a220ae809555cad86fb98d823ec77528a2fb125/modules/es/fsw/src/cfe_es_generic_pool.c#L607 https://github.com/nasa/cFE/blob/7a220ae809555cad86fb98d823ec77528a2fb125/modules/es/fsw/src/cfe_es_generic_pool.c#L626

CFE_ES_GenPoolGetBucketUsage() cannot currently be amended to have a const CFE_ES_GenPoolRecord_t input, as line 655 generates new warnings (about discarding the const qualifier). Casting as const may remove the warnings, but I don't think it's worth making additional changes to the function in order to add const qualifiers in the function parameters. https://github.com/nasa/cFE/blob/7a220ae809555cad86fb98d823ec77528a2fb125/modules/es/fsw/src/cfe_es_generic_pool.c#L655