On Mon, May 05, 2014 at 04:09:51PM +0200, Alvaro Neira Ayuso wrote:
With this patch, the manager monitors the sensors and send OML Failure message from the Manager to the BTS and the BTS to BSC, for having a report system of the sensors.
Sorry but the above is not too clear. Please try again.
+static struct osmo_timer_list connect_timer; +static void socket_connect_cb(void *data) +{
- fd_unix = osmo_sock_unix_init(SOCK_SEQPACKET, 0, SOCKET_PATH,
OSMO_SOCK_F_CONNECT);- if (fd_unix < 0) {
osmo_timer_schedule(&connect_timer, CONNECT_TIMER_SECS, 0);
What prevents socket_connect_cb(NULL) be called _while_ the timer is pending? While timers for check_temperature and socket_connect_cb might match now one should make sure the timer is stopped.
+static int check_temperature(struct uc *ucontrol0, int lowlimit, int highlimit, @@ -169,6 +260,7 @@ static void signal_handler(int signal) case SIGINT: sysmobts_check_temp(no_eeprom_write); sysmobts_update_hours(no_eeprom_write);
exit(0);close(fd_unix);
Well, if we close it we should set fd_unix to -1 and unregister the fd. This might not be signal-safe, so why to close it at all?
- char file_version[255];
- char file_id[255];
- strncpy(file_id, "sysmomgr", sizeof("sysmomgr"));
- file_id[sizeof(file_id) - 1] = '\0';
- strncpy(file_version, PACKAGE_VERSION, sizeof(PACKAGE_VERSION));
- file_version[sizeof(file_version) - 1] = '\0';
- msgb_v_put(msg, NM_ATT_SW_DESCR);
- msgb_tl16v_put(msg, NM_ATT_FILE_ID, strlen(file_id),
(uint8_t *)file_id);- msgb_tl16v_put(msg, NM_ATT_FILE_VERSION, strlen(file_version),
(uint8_t *)file_version);
That is odd. You use "sizeof" which includes the NUL and then you use strlen on file_id which will remove the NUL. This appears to be not consistent.
- if (is_manuf) {
/* length byte, string + 0 termination */uint8_t *manuf = msgb_push(msg, 1 + sizeof(osmocom_magic));manuf[0] = strlen(osmocom_magic)+1;memcpy(manuf+1, osmocom_magic, strlen(osmocom_magic));- }
The sizes look odd here. Sure you want to have one byte for the length, the string and the NUL byte but you expect/assume that the msg is NUL at strlen(osmocom_magic)+1. Maybe it is better to not make this assumption?
- strncpy(add_info->name_sensor, nsensor, sizeof(add_info->name_sensor));
- /* If we need to send this structure to other machine, we need to pass
* the integer inside the structure to internet format (big endian)*/- msgb_tl16v_put(msg, NM_ATT_ADD_INFO,
sizeof(struct sbts2050_config_info),(const uint8_t *)add_info);
yes? but it is sent to a BSC which might be another machine? Is this relevant?