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/.
Max msuraev at sysmocom.deYes, it's just rewrite of a cleanup from before which was not accepted because it had booleans in function signature so it's safe to just drop. The actual fix for a bug is in separate patch which does not depend on this one. Before fixing a bug I've got to first understand what existing code does which is not always immediately clear, especially if comments are missing. While going over a code I sometimes change/simplify functions more as a "note to self" to make sure I easier understand particular part of the code when getting back to it. If I feel that such changes might be generally useful than I send it as a patch. Of course such feeling is not always 100% match with what others feel so not all the patches go through. On 04/25/2016 03:03 PM, Harald Welte wrote: > Hi Max, > > On Mon, Apr 25, 2016 at 12:22:17PM +0200, msuraev at sysmocom.de wrote: >> * Explicitly use gsm_chan_t enum when checking for full rate channels. >> * Consistently use enums instead of (u)int, char etc. > Is this supposed to be a pure cleanup patch (not changing behavior) or a > bug fix (changing behavior)? This is currently not clear to me > > I'm not particularly happy that in many cases when there is a bug to > fix, you first seem to restructure/cleanup the code and then address the > bug. This adds the risk of introducing behavioral changes in the first > step (particularly with lack of good test coverage), and it also means > we're focussing less on the relevant topic. > >> Note: because actual enum representation is up to compiler this might >> change the size of gsm_mncc struct so the MNCC protocol version is >> bumped. > I think this kind of change just creates more effort for everyone and I > don't really see much benefit of it. You would have to merge related > changes not only to the NITB but to all its users (lcr, mncc_python, > osmo-sip-connector, ...) at the same time. Have you changed all the > respective users and re-tested them? Do you think it is a good use of > our time to do so for a merely stylistic improvement? > > In general I think it is a bad idea to have enum's in structures that > describe an external interface / protocol betweeo two programs, exactly > for the reason you stated. Alignment differences can be easily avoided > with __attribute__ ((packed)), but enums have the inherent problem of > '-fshort-enums', which might be default on some platform. and then you > add a new enum value, and your structure size changes again. > > Regards, > Harald -- Max Suraev <msuraev at sysmocom.de> http://www.sysmocom.de/ ======================================================================= * sysmocom - systems for mobile communications GmbH * Alt-Moabit 93 * 10559 Berlin, Germany * Sitz / Registered office: Berlin, HRB 134158 B * Geschaeftsfuehrer / Managing Directors: Holger Freyther, Harald Welte