Max has submitted this change and it was merged.

Change subject: Move parameter file opening into separate function
......................................................................


Move parameter file opening into separate function

* use talloc for file path allocation
* print detailed error on failures

This simplifies troubleshooting of lc15bts-mgr failures.

Change-Id: I86c93a2a4f080e8ac1517be93f58f6ffd00d248c
Related: SYS#3686
---
M src/osmo-bts-litecell15/misc/lc15bts_mgr_nl.c
M src/osmo-bts-litecell15/misc/lc15bts_misc.c
M src/osmo-bts-litecell15/misc/lc15bts_par.c
M src/osmo-bts-litecell15/misc/lc15bts_par.h
M src/osmo-bts-litecell15/misc/lc15bts_util.c
5 files changed, 52 insertions(+), 61 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/osmo-bts-litecell15/misc/lc15bts_mgr_nl.c 
b/src/osmo-bts-litecell15/misc/lc15bts_mgr_nl.c
index 903c5fe..549c179 100644
--- a/src/osmo-bts-litecell15/misc/lc15bts_mgr_nl.c
+++ b/src/osmo-bts-litecell15/misc/lc15bts_mgr_nl.c
@@ -116,7 +116,7 @@
                }
 
                /* fetch the serial number */
-               lc15bts_par_get_int(LC15BTS_PAR_SERNR, &serno);
+               lc15bts_par_get_int(tall_mgr_ctx, LC15BTS_PAR_SERNR, &serno);
                snprintf(ser_str, sizeof(ser_str), "%d", serno);
 
                /* fetch the model and trx number */
diff --git a/src/osmo-bts-litecell15/misc/lc15bts_misc.c 
b/src/osmo-bts-litecell15/misc/lc15bts_misc.c
index 5ff8e31..fa59b7c 100644
--- a/src/osmo-bts-litecell15/misc/lc15bts_misc.c
+++ b/src/osmo-bts-litecell15/misc/lc15bts_misc.c
@@ -114,12 +114,13 @@
 
        for (i = 0; i < ARRAY_SIZE(temp_data); i++) {
                int ret;
-               rc = lc15bts_par_get_int(temp_data[i].ee_par, &ret);
+               rc = lc15bts_par_get_int(tall_mgr_ctx, temp_data[i].ee_par, 
&ret);
                temp_old[i] = ret * 1000;
 
                temp_cur[i] = lc15bts_temp_get(temp_data[i].sensor);
                if (temp_cur[i] < 0 && temp_cur[i] > -1000) {
-                       LOGP(DTEMP, LOGL_ERROR, "Error reading temperature 
(%d)\n", temp_data[i].sensor);
+                       LOGP(DTEMP, LOGL_ERROR, "Error reading temperature 
(%d): unexpected value %d\n",
+                            temp_data[i].sensor, temp_cur[i]);
                        continue;
                }
        
@@ -132,8 +133,7 @@
                             temp_cur[i]/1000, temp_old[i]%1000);
 
                        if (!no_rom_write) {
-                               rc = lc15bts_par_set_int(temp_data[i].ee_par,
-                                                 temp_cur[i]/1000);
+                               rc = lc15bts_par_set_int(tall_mgr_ctx, 
temp_data[i].ee_par, temp_cur[i]/1000);
                                if (rc < 0)
                                        LOGP(DTEMP, LOGL_ERROR, "error writing 
new %s "
                                             "max temp %d (%s)\n", 
temp_data[i].name,
@@ -157,7 +157,7 @@
        if (last_update == 0) {
                last_update = now;
 
-               rc = lc15bts_par_get_int(LC15BTS_PAR_HOURS, &op_hrs);
+               rc = lc15bts_par_get_int(tall_mgr_ctx, LC15BTS_PAR_HOURS, 
&op_hrs);
                if (rc < 0) {
                        LOGP(DTEMP, LOGL_ERROR, "Unable to read "
                             "operational hours: %d (%s)\n", rc,
@@ -172,7 +172,7 @@
        }
 
        if (now >= last_update + 3600) {
-               rc = lc15bts_par_get_int(LC15BTS_PAR_HOURS, &op_hrs);
+               rc = lc15bts_par_get_int(tall_mgr_ctx, LC15BTS_PAR_HOURS, 
&op_hrs);
                if (rc < 0) {
                        LOGP(DTEMP, LOGL_ERROR, "Unable to read "
                             "operational hours: %d (%s)\n", rc,
@@ -187,7 +187,7 @@
                     op_hrs);
 
                if (!no_rom_write) {
-                       rc = lc15bts_par_set_int(LC15BTS_PAR_HOURS, op_hrs);
+                       rc = lc15bts_par_set_int(tall_mgr_ctx, 
LC15BTS_PAR_HOURS, op_hrs);
                        if (rc < 0)
                                return rc;
                }
diff --git a/src/osmo-bts-litecell15/misc/lc15bts_par.c 
b/src/osmo-bts-litecell15/misc/lc15bts_par.c
index 3d80e67..ab3e96b 100644
--- a/src/osmo-bts-litecell15/misc/lc15bts_par.c
+++ b/src/osmo-bts-litecell15/misc/lc15bts_par.c
@@ -35,12 +35,9 @@
 #include <sys/stat.h>
 
 #include <osmocom/core/utils.h>
+#include <osmocom/core/talloc.h>
 
 #include "lc15bts_par.h"
-
-
-#define FACTORY_ROM_PATH       "/mnt/rom/factory"
-#define USER_ROM_PATH          "/mnt/rom/user"
 
 const struct value_string lc15bts_par_names[_NUM_LC15BTS_PAR+1] = {
        { LC15BTS_PAR_TEMP_SUPPLY_MAX,  "temp-supply-max" },
@@ -80,19 +77,32 @@
        }
 }
 
-int lc15bts_par_get_int(enum lc15bts_par par, int *ret)
+FILE *lc15bts_par_get_path(void *ctx, enum lc15bts_par par, const char* mode)
 {
-       char fpath[PATH_MAX];
+       char *fpath;
        FILE *fp;
-       int rc;
 
        if (par >= _NUM_LC15BTS_PAR)
-               return -ENODEV;
+               return NULL;
 
-       snprintf(fpath, sizeof(fpath)-1, "%s/%s", USER_ROM_PATH, 
get_value_string(lc15bts_par_names, par));
-       fpath[sizeof(fpath)-1] = '\0';
+       fpath = talloc_asprintf(ctx, "%s/%s", USER_ROM_PATH, 
get_value_string(lc15bts_par_names, par));
+       if (!fpath)
+               return NULL;
 
-       fp = fopen(fpath, "r");
+       fp = fopen(fpath, mode);
+       if (!fp)
+               fprintf(stderr, "Failed to open %s due to '%s' error\n", fpath, 
strerror(errno));
+
+       talloc_free(fpath);
+
+       return fp;
+}
+
+int lc15bts_par_get_int(void *ctx, enum lc15bts_par par, int *ret)
+{
+       FILE *fp = lc15bts_par_get_path(ctx, par, "r");
+       int rc;
+
        if (fp == NULL) {
                return -errno;
        }
@@ -106,19 +116,11 @@
        return 0;
 }
 
-int lc15bts_par_set_int(enum lc15bts_par par, int val)
+int lc15bts_par_set_int(void *ctx, enum lc15bts_par par, int val)
 {
-       char fpath[PATH_MAX];
-       FILE *fp;
+       FILE *fp = lc15bts_par_get_path(ctx, par, "w");
        int rc;
 
-       if (par >= _NUM_LC15BTS_PAR)
-               return -ENODEV;
-
-       snprintf(fpath, sizeof(fpath)-1, "%s/%s", USER_ROM_PATH, 
get_value_string(lc15bts_par_names, par));
-       fpath[sizeof(fpath)-1] = '\0';
-
-       fp = fopen(fpath, "w");
        if (fp == NULL) {
                return -errno;
        }
@@ -132,20 +134,11 @@
        return 0;
 }
 
-int lc15bts_par_get_buf(enum lc15bts_par par, uint8_t *buf,
-                        unsigned int size)
+int lc15bts_par_get_buf(void *ctx, enum lc15bts_par par, uint8_t *buf, 
unsigned int size)
 {      
-       char fpath[PATH_MAX];
-       FILE *fp;
+       FILE *fp = lc15bts_par_get_path(ctx, par, "rb");
        int rc;
 
-       if (par >= _NUM_LC15BTS_PAR)
-               return -ENODEV;
-
-       snprintf(fpath, sizeof(fpath)-1, "%s/%s", USER_ROM_PATH, 
get_value_string(lc15bts_par_names, par));
-       fpath[sizeof(fpath)-1] = '\0';
-
-       fp = fopen(fpath, "rb");
        if (fp == NULL) {
                return -errno;
        }
@@ -157,20 +150,11 @@
        return rc;
 }
 
-int lc15bts_par_set_buf(enum lc15bts_par par, const uint8_t *buf,
-                        unsigned int size)
+int lc15bts_par_set_buf(void *ctx, enum lc15bts_par par, const uint8_t *buf, 
unsigned int size)
 {
-        char fpath[PATH_MAX];
-        FILE *fp;
+        FILE *fp = lc15bts_par_get_path(ctx, par, "wb");
         int rc;
 
-        if (par >= _NUM_LC15BTS_PAR)
-                return -ENODEV;
-
-        snprintf(fpath, sizeof(fpath)-1, "%s/%s", USER_ROM_PATH, 
get_value_string(lc15bts_par_names, par));
-        fpath[sizeof(fpath)-1] = '\0';
-
-        fp = fopen(fpath, "wb");
         if (fp == NULL) {
                 return -errno;
        }
diff --git a/src/osmo-bts-litecell15/misc/lc15bts_par.h 
b/src/osmo-bts-litecell15/misc/lc15bts_par.h
index c50a69f..b9fe740 100644
--- a/src/osmo-bts-litecell15/misc/lc15bts_par.h
+++ b/src/osmo-bts-litecell15/misc/lc15bts_par.h
@@ -3,6 +3,9 @@
 
 #include <osmocom/core/utils.h>
 
+#define FACTORY_ROM_PATH       "/mnt/rom/factory"
+#define USER_ROM_PATH          "/mnt/rom/user"
+
 enum lc15bts_par {
        LC15BTS_PAR_TEMP_SUPPLY_MAX,
        LC15BTS_PAR_TEMP_SOC_MAX,
@@ -22,12 +25,10 @@
 
 extern const struct value_string lc15bts_par_names[_NUM_LC15BTS_PAR+1];
 
-int lc15bts_par_get_int(enum lc15bts_par par, int *ret);
-int lc15bts_par_set_int(enum lc15bts_par par, int val);
-int lc15bts_par_get_buf(enum lc15bts_par par, uint8_t *buf,
-                        unsigned int size);
-int lc15bts_par_set_buf(enum lc15bts_par par, const uint8_t *buf,
-                        unsigned int size);
+int lc15bts_par_get_int(void *ctx, enum lc15bts_par par, int *ret);
+int lc15bts_par_set_int(void *ctx, enum lc15bts_par par, int val);
+int lc15bts_par_get_buf(void *ctx, enum lc15bts_par par, uint8_t *buf, 
unsigned int size);
+int lc15bts_par_set_buf(void *ctx, enum lc15bts_par par, const uint8_t *buf, 
unsigned int size);
 
 int lc15bts_par_is_int(enum lc15bts_par par);
 
diff --git a/src/osmo-bts-litecell15/misc/lc15bts_util.c 
b/src/osmo-bts-litecell15/misc/lc15bts_util.c
index 33f9e4e..430ce0f 100644
--- a/src/osmo-bts-litecell15/misc/lc15bts_util.c
+++ b/src/osmo-bts-litecell15/misc/lc15bts_util.c
@@ -28,9 +28,12 @@
 #include <string.h>
 #include <errno.h>
 #include <getopt.h>
-
+#include <osmocom/core/talloc.h>
+#include <osmocom/core/msgb.h>
 
 #include "lc15bts_par.h"
+
+void *tall_util_ctx;
 
 enum act {
        ACT_GET,
@@ -101,6 +104,9 @@
        enum lc15bts_par par;
        int rc, val;
 
+       tall_util_ctx = talloc_named_const(NULL, 1, "lc15 utils");
+       msgb_talloc_ctx_init(tall_util_ctx, 0);
+
        rc = parse_options(argc, argv);
        if (rc < 0)
                exit(2);
@@ -120,7 +126,7 @@
 
        switch (action) {
        case ACT_GET:
-               rc = lc15bts_par_get_int(par, &val);
+               rc = lc15bts_par_get_int(tall_util_ctx, par, &val);
                if (rc < 0) {
                        fprintf(stderr, "Error %d\n", rc);
                        goto err;
@@ -128,7 +134,7 @@
                printf("%d\n", val);
                break;
        case ACT_SET:
-               rc = lc15bts_par_get_int(par, &val);
+               rc = lc15bts_par_get_int(tall_util_ctx, par, &val);
                if (rc < 0) {
                        fprintf(stderr, "Error %d\n", rc);
                        goto err;
@@ -137,7 +143,7 @@
                        fprintf(stderr, "Parameter is already set!\r\n");
                        goto err;
                }
-               rc = lc15bts_par_set_int(par, atoi(write_arg));
+               rc = lc15bts_par_set_int(tall_util_ctx, par, atoi(write_arg));
                if (rc < 0) {
                        fprintf(stderr, "Error %d\n", rc);
                        goto err;

-- 
To view, visit https://gerrit.osmocom.org/2928
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I86c93a2a4f080e8ac1517be93f58f6ffd00d248c
Gerrit-PatchSet: 2
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Max <msur...@sysmocom.de>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msur...@sysmocom.de>

Reply via email to