From patchwork Sat Jan 2 19:42:33 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Vadim Yanitskiy X-Patchwork-Id: 562101 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.osmocom.org (tmp.osmocom.org [144.76.43.76]) by ozlabs.org (Postfix) with ESMTP id BE9511402EC for ; Sun, 3 Jan 2016 06:42:39 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=yRoEh6cB; dkim-atps=neutral Received: from lists.osmocom.org (lists.osmocom.org [144.76.43.76]) by lists.osmocom.org (Postfix) with ESMTP id 8C92CBF8B; Sat, 2 Jan 2016 19:42:37 +0000 (UTC) X-Original-To: baseband-devel@lists.osmocom.org Delivered-To: baseband-devel@lists.osmocom.org Received: from mail-ob0-x22c.google.com (mail-ob0-x22c.google.com [IPv6:2607:f8b0:4003:c01::22c]) by lists.osmocom.org (Postfix) with ESMTP id 701D0BF6C for ; Sat, 2 Jan 2016 19:42:34 +0000 (UTC) Received: by mail-ob0-x22c.google.com with SMTP id 18so317639935obc.2 for ; Sat, 02 Jan 2016 11:42:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; bh=Q0bZF/bKyl+srjcwtK6nUUtsL7lIzG3pXsg4tQ3wQQk=; b=yRoEh6cB0fmgMyWgGWWmxez2BIPIFB2sRVosmBsIamyERwvnMTqdEu+r9uoop0etop MrcxRLYk8bOJ3EwQUZS8anXryx0V4+sVCiYAWhLwKgcm0mUNWTQ9gnqVVdFJ3UF5yghs r5rDKJ3YtgeD1WSpCPwi77AglmLJ64ynXahOnYGA5DDHIwEDIoNWFj6mr1x+gOzAYQTT qZI5qhplMaaYyfrmBiWVT351Sn4bUisZ1obXGq+5Dqp0D5aBgVAK9F53P4TUUi6L1yMn VywuJwDVXZ0KW1c5yoDVk/hueGwUUx0prr/eA7KeuStIvoP9jUVsWBKCe4AzW/2ctdRE PWdg== MIME-Version: 1.0 X-Received: by 10.182.28.33 with SMTP id y1mr46178767obg.53.1451763753548; Sat, 02 Jan 2016 11:42:33 -0800 (PST) Received: by 10.202.198.148 with HTTP; Sat, 2 Jan 2016 11:42:33 -0800 (PST) In-Reply-To: References: <20160102191957.GB4704@nataraja> Date: Sun, 3 Jan 2016 01:42:33 +0600 Message-ID: Subject: Re: [PATCH] mobile_app: fix some configuration r/w bugs From: =?UTF-8?B?0JLQsNC00LjQvCDQr9C90LjRhtC60LjQuQ==?= To: baseband-devel@lists.osmocom.org X-BeenThere: baseband-devel@lists.osmocom.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: OsmocomBB - open source GSM baseband firmware List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: baseband-devel-bounces@lists.osmocom.org Sender: "baseband-devel" Sorry, it was a Copy/Paste style mistake. Fixed. { @@ -1075,7 +1075,7 @@ DEFUN(cfg_gps_host, cfg_gps_host_cmd, "gps host HOST:PORT", if (colon != NULL) { memcpy(g.gpsd_host, argv[0], colon - argv[0] - 1); g.gpsd_host[colon - argv[0]] = '\0'; - memcpy(g.gpsd_port, colon, strlen(colon)); + memcpy(g.gpsd_port, colon + 1, strlen(colon)); g.gpsd_port[strlen(colon)] = '\0'; } else { snprintf(g.gpsd_host, ARRAY_SIZE(g.gpsd_host), "%s", argv[0]); @@ -1499,8 +1499,7 @@ static int config_write(struct vty *vty) struct osmocom_ms *ms; #ifdef _HAVE_GPSD - vty_out(vty, "gpsd host %s%s", g.gpsd_host, VTY_NEWLINE); - vty_out(vty, "gpsd port %s%s", g.gpsd_port, VTY_NEWLINE); + vty_out(vty, "gpsd host %s:%s%s", g.gpsd_host, g.gpsd_port, VTY_NEWLINE); #endif vty_out(vty, "gps device %s%s", g.device, VTY_NEWLINE); if (g.baud) @@ -2822,7 +2821,7 @@ int ms_vty_init(void) install_element(ENABLE_NODE, &delete_forbidden_plmn_cmd); #ifdef _HAVE_GPSD - install_element(CONFIG_NODE, &cfg_gps_host_cmd); + install_element(CONFIG_NODE, &cfg_gpsd_host_cmd); #endif install_element(CONFIG_NODE, &cfg_gps_device_cmd); install_element(CONFIG_NODE, &cfg_gps_baud_cmd); 2016-01-03 1:38 GMT+06:00 Вадим Яницкий : > Sorry, was a Copy/Paste mistake. Fixed. > > diff --git a/src/host/layer23/include/osmocom/bb/common/sap_interface.h > b/src/host/layer23/include/osmocom/bb/common/sap_interface.h > index bf19356..e4e64ce 100644 > --- a/src/host/layer23/include/osmocom/bb/common/sap_interface.h > +++ b/src/host/layer23/include/osmocom/bb/common/sap_interface.h > @@ -11,6 +11,7 @@ int osmosap_sapsocket(struct osmocom_ms *ms, const char > *path); > int osmosap_init(struct osmocom_ms *ms); > > enum osmosap_state { > + SAP_SOCKET_ERROR, > SAP_NOT_CONNECTED, > SAP_IDLE, > SAP_CONNECTION_UNDER_NEGOTIATION, > diff --git a/src/host/layer23/src/common/sap_interface.c > b/src/host/layer23/src/common/sap_interface.c > index a56f4f2..d5e8d1d 100644 > --- a/src/host/layer23/src/common/sap_interface.c > +++ b/src/host/layer23/src/common/sap_interface.c > @@ -515,7 +515,7 @@ int sap_open(struct osmocom_ms *ms, const char > *socket_path) > rc = connect(ms->sap_wq.bfd.fd, (struct sockaddr *) &local, > sizeof(local)); > if (rc < 0) { > fprintf(stderr, "Failed to connect to '%s'\n", local.sun_path); > - set->sap_socket_path[0] = 0; > + ms->sap_entity.sap_state = SAP_SOCKET_ERROR; > > close(ms->sap_wq.bfd.fd); > return rc; > } > diff --git a/src/host/layer23/src/common/sim.c > b/src/host/layer23/src/common/sim.c > index 8e8d7bf..df9fbd2 100644 > --- a/src/host/layer23/src/common/sim.c > +++ b/src/host/layer23/src/common/sim.c > @@ -188,7 +188,7 @@ static int sim_apdu_send(struct osmocom_ms *ms, > uint8_t *data, uint16_t length) > > /* adding SAP client support > * it makes more sense to do it here then in L1CTL */ > - if(ms->settings.sap_socket_path[0] == 0) { > + if(ms->sap_entity.sap_state == SAP_SOCKET_ERROR) { > LOGP(DSIM, LOGL_INFO, "Using built-in SIM reader\n"); > l1ctl_tx_sim_req(ms, data, length); > } else { > diff --git a/src/host/layer23/src/mobile/vty_interface.c > b/src/host/layer23/src/mobile/vty_interface.c > index 5782a17..fd0fd0c 100644 > --- a/src/host/layer23/src/mobile/vty_interface.c > +++ b/src/host/layer23/src/mobile/vty_interface.c > @@ -1067,7 +1067,7 @@ DEFUN(cfg_no_gps_enable, cfg_no_gps_enable_cmd, "no > gps enable", > } > > #ifdef _HAVE_GPSD > -DEFUN(cfg_gps_host, cfg_gps_host_cmd, "gps host HOST:PORT", > +DEFUN(cfg_gpsd_host, cfg_gpsd_host_cmd, "gpsd host HOST:PORT", > "GPS receiver\nSelect gpsd host and port\n" > "IP and port (optional) of the host running gpsd") > { > @@ -1075,7 +1075,7 @@ DEFUN(cfg_gps_host, cfg_gps_host_cmd, "gps host > HOST:PORT", > if (colon != NULL) { > memcpy(g.gpsd_host, argv[0], colon - argv[0] - 1); > g.gpsd_host[colon - argv[0]] = '\0'; > - memcpy(g.gpsd_port, colon, strlen(colon)); > + memcpy(g.gpsd_port, colon + 1, strlen(colon)); > g.gpsd_port[strlen(colon)] = '\0'; > } else { > snprintf(g.gpsd_host, ARRAY_SIZE(g.gpsd_host), "%s", argv[0]); > @@ -1499,8 +1499,7 @@ static int config_write(struct vty *vty) > struct osmocom_ms *ms; > > #ifdef _HAVE_GPSD > - vty_out(vty, "gpsd host %s%s", g.gpsd_host, VTY_NEWLINE); > - vty_out(vty, "gpsd port %s%s", g.gpsd_port, VTY_NEWLINE); > + vty_out(vty, "gpsd host %s:%s%s", g.gpsd_host, g.gpsd_port, > VTY_NEWLINE); > #endif > vty_out(vty, "gps device %s%s", g.device, VTY_NEWLINE); > if (g.baud) > @@ -2822,7 +2821,7 @@ int ms_vty_init(void) > install_element(ENABLE_NODE, &delete_forbidden_plmn_cmd); > > #ifdef _HAVE_GPSD > - install_element(CONFIG_NODE, &cfg_gps_host_cmd); > + install_element(CONFIG_NODE, &cfg_gpsd_host_cmd); > #endif > install_element(CONFIG_NODE, &cfg_gps_device_cmd); > install_element(CONFIG_NODE, &cfg_gps_baud_cmd); > > > 2016-01-03 1:19 GMT+06:00 Harald Welte : > >> Hi Vadim, >> >> thanks for your patch, but.. >> >> On Sun, Jan 03, 2016 at 01:14:23AM +0600, Вадим Яницкий wrote: >> > - set->sap_socket_path[0] = 0; >> > + // set->sap_socket_path[0] = 0; >> > + ms->sap_entity.sap_state == SAP_SOCKET_ERROR; >> >> a) if you remove code, remove it for good, not just comment it out >> >> b) the double-equals will not cause an assignment. I don't think you >> tested this code... >> >> Regards, >> Harald >> -- >> - Harald Welte >> http://laforge.gnumonks.org/ >> >> ============================================================================ >> "Privacy in residential applications is a desirable marketing option." >> (ETSI EN 300 175-7 Ch. >> A6) >> > > diff --git a/src/host/layer23/include/osmocom/bb/common/sap_interface.h b/src/host/layer23/include/osmocom/bb/common/sap_interface.h index bf19356..e4e64ce 100644 --- a/src/host/layer23/include/osmocom/bb/common/sap_interface.h +++ b/src/host/layer23/include/osmocom/bb/common/sap_interface.h @@ -11,6 +11,7 @@ int osmosap_sapsocket(struct osmocom_ms *ms, const char *path); int osmosap_init(struct osmocom_ms *ms); enum osmosap_state { + SAP_SOCKET_ERROR, SAP_NOT_CONNECTED, SAP_IDLE, SAP_CONNECTION_UNDER_NEGOTIATION, diff --git a/src/host/layer23/src/common/sap_interface.c b/src/host/layer23/src/common/sap_interface.c index a56f4f2..d5e8d1d 100644 --- a/src/host/layer23/src/common/sap_interface.c +++ b/src/host/layer23/src/common/sap_interface.c @@ -515,7 +515,7 @@ int sap_open(struct osmocom_ms *ms, const char *socket_path) rc = connect(ms->sap_wq.bfd.fd, (struct sockaddr *) &local, sizeof(local)); if (rc < 0) { fprintf(stderr, "Failed to connect to '%s'\n", local.sun_path); - set->sap_socket_path[0] = 0; + ms->sap_entity.sap_state = SAP_SOCKET_ERROR; close(ms->sap_wq.bfd.fd); return rc; } diff --git a/src/host/layer23/src/common/sim.c b/src/host/layer23/src/common/sim.c index 8e8d7bf..df9fbd2 100644 --- a/src/host/layer23/src/common/sim.c +++ b/src/host/layer23/src/common/sim.c @@ -188,7 +188,7 @@ static int sim_apdu_send(struct osmocom_ms *ms, uint8_t *data, uint16_t length) /* adding SAP client support * it makes more sense to do it here then in L1CTL */ - if(ms->settings.sap_socket_path[0] == 0) { + if(ms->sap_entity.sap_state == SAP_SOCKET_ERROR) { LOGP(DSIM, LOGL_INFO, "Using built-in SIM reader\n"); l1ctl_tx_sim_req(ms, data, length); } else { diff --git a/src/host/layer23/src/mobile/vty_interface.c b/src/host/layer23/src/mobile/vty_interface.c index 5782a17..fd0fd0c 100644 --- a/src/host/layer23/src/mobile/vty_interface.c +++ b/src/host/layer23/src/mobile/vty_interface.c @@ -1067,7 +1067,7 @@ DEFUN(cfg_no_gps_enable, cfg_no_gps_enable_cmd, "no gps enable", } #ifdef _HAVE_GPSD -DEFUN(cfg_gps_host, cfg_gps_host_cmd, "gps host HOST:PORT", +DEFUN(cfg_gpsd_host, cfg_gpsd_host_cmd, "gpsd host HOST:PORT", "GPS receiver\nSelect gpsd host and port\n" "IP and port (optional) of the host running gpsd")