two general code review questions

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.

Harald Welte laforge at gnumonks.org
Fri Jan 18 09:58:06 UTC 2019


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_iface.h#2

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_server.h#18
> 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 <laforge at gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)



More information about the OpenBSC mailing list