Should we now put copyright comments in header files as well? re https://gerrit.osmocom.org/c/osmo-msc/+/11642/3/include/osmocom/msc/sgs_ifac...
When I see structs containing talloc'd char*, should I generally give code review to use fixed char arrays instead? I think it ends up being simpler semantically as well as better in terms of mem allocation. Right? e.g. https://gerrit.osmocom.org/c/osmo-msc/+/11642/30/include/osmocom/msc/sgs_ser... and #21
Thanks for your opinions...
~N
On Thu, Jan 17, 2019 at 05:50:05PM +0100, Neels Hofmeyr wrote:
Should we now put copyright comments in header files as well? re https://gerrit.osmocom.org/c/osmo-msc/+/11642/3/include/osmocom/msc/sgs_ifac...
When I see structs containing talloc'd char*, should I generally give code review to use fixed char arrays instead? I think it ends up being simpler semantically as well as better in terms of mem allocation. Right? e.g. https://gerrit.osmocom.org/c/osmo-msc/+/11642/30/include/osmocom/msc/sgs_ser... and #21
From IRC:
<LaF0rge> neels: it depends. if the size is bounded, and we always need that string as part of the enclosing struct: array. <LaF0rge> neels: if the size is very dynamic/unknown/unbounded, and the char string is rather optional and not present in most cases: dynamic allocation
~N
Hi Neels,
On Thu, Jan 17, 2019 at 05:50:05PM +0100, Neels Hofmeyr wrote:
Should we now put copyright comments in header files as well? re https://gerrit.osmocom.org/c/osmo-msc/+/11642/3/include/osmocom/msc/sgs_ifac...
yes, it is general practise.
When I see structs containing talloc'd char*, should I generally give code review to use fixed char arrays instead? I think it ends up being simpler semantically as well as better in terms of mem allocation. Right? e.g. https://gerrit.osmocom.org/c/osmo-msc/+/11642/30/include/osmocom/msc/sgs_ser... and #21
My general rule of thumb is: * if the string length is [relatively] bounded and the most common case is that the enclosing structure will have such a string present -> static array * if the string length is rather unbounded and/or there are many use cases where the string is not used at all -> pointer + talloc
There may be other factors to take into consideration. For strings that can change at the lietime of the structure, such as 'name' or 'description' bits that are accessible from the VTY, we've traditionally used osmo_talloc_replace_string() as a hepler and then there's some tendency to do that. But then, if it were a static array, we'd simply use OSMO_STRLCPY_ARRAY() which is equally easy to use.
Regards, Harald
-
Harald Welte -
Neels Hofmeyr