[PATCH 0/2] device_script() reimplementation

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2] device_script() reimplementation

Ladislav Michl
Hi there,

as Fabrizio Gennari is now trying to revive win32 support, let's take an
opportunity and rewrite (dis)connect_scripts support to be more widely
useable, less resource hungry and preferrably ifdefless.

So here's an attempt doing that. Not yet finished and compile tested only.
Please speak out loud if you do not like this solution or it does not
work on your platform. Patch is premiliary, just to show an idea.

Thank you,
        ladis

_______________________________________________
gnokii-users mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/gnokii-users
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] Call connect_script only for opened device

Ladislav Michl
Previously connect_script was called only with valid fd, restore that.

Fixes: 4d38aee466e6 (Make device_script() generic for all connection types)
Signed-off-by: Ladislav Michl <[hidden email]>
---
 common/device.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/common/device.c b/common/device.c
index 1b1daabb..794106b7 100644
--- a/common/device.c
+++ b/common/device.c
@@ -125,6 +125,10 @@ int device_open(const char *file, int with_odd_parity, int with_async,
  state->device.fd = -1;
  break;
  }
+
+ if (state->device.fd < 0)
+ return 0;
+
  /*
  * handle config file connect_script:
  */
@@ -134,7 +138,7 @@ int device_open(const char *file, int with_odd_parity, int with_async,
  return 0;
  }
 
- return (state->device.fd >= 0);
+ return 1;
 }
 
 void device_close(struct gn_statemachine *state)
--
2.19.1


_______________________________________________
gnokii-users mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/gnokii-users
Reply | Threaded
Open this post in threaded view
|

[PATCH v0 2/2] Use posix_spawn to tun external scripts

Ladislav Michl
In reply to this post by Ladislav Michl
posix_spawn specification dates back to last century and its
implementation is mature enough in all systems we do support.
Thus use it instead of current fork and exec in hope it will
save us some resources.

Signed-off-by: Ladislav Michl <[hidden email]>
---
 !!! NOTE: This is an untested draft only !!!

 common/Makefile.am        |   8 ++-
 common/cfgreader.c        |  23 ---------
 common/device.c           |  59 +---------------------
 common/posixscript.c      | 102 ++++++++++++++++++++++++++++++++++++++
 common/winscript.c        |  36 ++++++++++++++
 configure.ac              |   2 +
 include/gnokii-internal.h |  16 +++++-
 7 files changed, 163 insertions(+), 83 deletions(-)
 create mode 100644 common/posixscript.c
 create mode 100644 common/winscript.c

diff --git a/common/Makefile.am b/common/Makefile.am
index 597ba7dc..0933f100 100644
--- a/common/Makefile.am
+++ b/common/Makefile.am
@@ -2,10 +2,15 @@ DEFS = -DLOCALEDIR=\"$(localedir)\" -DCOMPILING_LIBGNOKII
 
 if WIN32
 DATA_DIR =
+DEVICE_SCRIPT = winscript.c
 else
 DATA_DIR = data
 endif
 
+if HAVE_POSIX_SPAWN
+DEVICE_SCRIPT = posixscript.c
+endif
+
 lib_LTLIBRARIES = libgnokii.la
 
 SUBDIRS = phones \
@@ -51,7 +56,8 @@ libgnokii_la_SOURCES = \
  snprintf.c \
  localcharset.c \
  map.c \
- gsm-auth.c
+ gsm-auth.c \
+ $(DEVICE_SCRIPT)
 
 libgnokii_la_LIBADD = \
  $(top_builddir)/common/phones/libPHONES.la \
diff --git a/common/cfgreader.c b/common/cfgreader.c
index abf2ae4e..ee1b4c3e 100644
--- a/common/cfgreader.c
+++ b/common/cfgreader.c
@@ -652,29 +652,6 @@ int cfg_section_exists(struct gn_cfg_header *cfg, const char *section)
  return false;
 }
 
-/*
- * Return all the entries of the given section.
- */
-void cfg_foreach(const char *section, cfg_foreach_func func)
-{
- struct gn_cfg_header *h;
- struct gn_cfg_entry *e;
- struct gn_cfg_header *cfg = gn_cfg_info;
-
- if ((cfg == NULL) || (section == NULL) || (func == NULL)) {
- return;
- }
-
- /* Search for section name */
- for (h = cfg; h != NULL; h = h->next) {
- if (strcmp(section, h->section) == 0) {
- /* Search for key within section */
- for (e = h->entries; e != NULL; e = e->next)
- (*func)(section, e->key, e->value);
- }
- }
-}
-
 /*  Set the value of a key in a config file.  Return the new value if
     the section/key can be found, else return NULL.  */
 char *cfg_set(struct gn_cfg_header *cfg, const char *section, const char *key,
diff --git a/common/device.c b/common/device.c
index 794106b7..3b4d2c3d 100644
--- a/common/device.c
+++ b/common/device.c
@@ -28,66 +28,11 @@
 #include "devices/dku2libusb.h"
 #include "devices/socketphonet.h"
 
-#include <errno.h>
-#include <sys/wait.h>
-
 GNOKII_API int device_getfd(struct gn_statemachine *state)
 {
  return state->device.fd;
 }
 
-/* Script handling: */
-static void device_script_cfgfunc(const char *section, const char *key, const char *value)
-{
- setenv(key, value, 1); /* errors ignored */
-}
-
-int device_script(int fd, const char *section, struct gn_statemachine *state)
-{
- pid_t pid;
- const char *scriptname;
- int status;
-
- if (!strcmp(section, "connect_script"))
- scriptname = state->config.connect_script;
- else
- scriptname = state->config.disconnect_script;
- if (scriptname[0] == '\0')
- return 0;
-
- errno = 0;
- switch ((pid = fork())) {
- case -1:
- fprintf(stderr, _("device_script(\"%s\"): fork() failure: %s!\n"), scriptname, strerror(errno));
- return -1;
-
- case 0: /* child */
- cfg_foreach(section, device_script_cfgfunc);
- errno = 0;
- if (dup2(fd, 0) != 0 || dup2(fd, 1) != 1 || close(fd)) {
- fprintf(stderr, _("device_script(\"%s\"): file descriptor preparation failure: %s\n"), scriptname, strerror(errno));
- _exit(-1);
- }
- /* FIXME: close all open descriptors - how to track them?
- */
- execl("/bin/sh", "sh", "-c", scriptname, NULL);
- fprintf(stderr, _("device_script(\"%s\"): script execution failure: %s\n"), scriptname, strerror(errno));
- _exit(-1);
- /* NOTREACHED */
-
- default:
- if (pid == waitpid(pid, &status, 0 /* options */) && WIFEXITED(status) && !WEXITSTATUS(status))
- return 0;
- fprintf(stderr, _("device_script(\"%s\"): child script execution failure: %s, exit code=%d\n"), scriptname,
- (WIFEXITED(status) ? _("normal exit") : _("abnormal exit")),
- (WIFEXITED(status) ? WEXITSTATUS(status) : -1));
- errno = EIO;
- return -1;
-
- }
- /* NOTREACHED */
-}
-
 int device_open(const char *file, int with_odd_parity, int with_async,
  int with_hw_handshake, gn_connection_type device_type,
  struct gn_statemachine *state)
@@ -132,7 +77,7 @@ int device_open(const char *file, int with_odd_parity, int with_async,
  /*
  * handle config file connect_script:
  */
- if (device_script(state->device.fd, "connect_script", state) == -1) {
+ if (device_script(state->device.fd, 1, state)) {
  dprintf("gnokii open device: connect_script failure\n");
  device_close(state);
  return 0;
@@ -148,7 +93,7 @@ void device_close(struct gn_statemachine *state)
  /*
  * handle config file disconnect_script:
  */
- if (device_script(state->device.fd, "disconnect_script", state) == -1)
+ if (device_script(state->device.fd, 0, state))
  dprintf("gnokii device close: disconnect_script failure\n");
 
  switch (state->device.type) {
diff --git a/common/posixscript.c b/common/posixscript.c
new file mode 100644
index 00000000..5079f898
--- /dev/null
+++ b/common/posixscript.c
@@ -0,0 +1,102 @@
+/*
+
+  G N O K I I
+
+  A Linux/Unix toolset and driver for the mobile phones.
+
+  This file is part of gnokii.
+
+  Copyright (C) 2018       Ladislav Michl
+
+*/
+
+#include "config.h"
+#include "compat.h"
+#include "misc.h"
+#include "gnokii.h"
+#include "gnokii-internal.h"
+
+#include <errno.h>
+#include <spawn.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int device_script(int fd, int connect, struct gn_statemachine *state)
+{
+ const char *scriptname, *section;
+ char **envp, *env;
+ struct gn_cfg_entry *e;
+ struct gn_cfg_header *h;
+ posix_spawn_file_actions_t fa;
+ pid_t pid;
+ int cnt, ret, status;
+ size_t len;
+
+ if (connect) {
+ scriptname = state->config.connect_script;
+ section = "connect_script";
+ } else {
+ scriptname = state->config.disconnect_script;
+ section = "disconnect_script";
+ }
+ if (scriptname[0] == '\0')
+ return 0;
+
+ cnt = 0; len = 0; ret = -1;
+ h = gn_cfg_info;
+ cfg_foreach_entry(section, h, e) {
+ len += strlen(e->key);
+ len += strlen(e->value);
+ len += 1 + 1 + sizeof(char *);
+ cnt++;
+ }
+
+ env = malloc(len + sizeof(char *));
+ if (!env) {
+ fprintf(stderr, _("device_script(\"%s\"): out of memory\n"), scriptname);
+ goto out;
+ }
+
+ envp = &env;
+ env += sizeof(char *) * (cnt + 1);
+ h = gn_cfg_info;
+ cfg_foreach_entry(section, h, e) {
+ *envp++ = &env;
+ env += snprintf(env, "%s=%s", e->key, e->value);
+ env++;
+ }
+ envp = NULL;
+
+ if (posix_spawn_file_actions_init(&fa)) {
+ fprintf(stderr, _("device_script(\"%s\"): file descriptor preparation failure: %s\n"),
+ scriptname, strerror(errno));
+ goto out_free;
+ }
+ if (posix_spawn_file_actions_adddup2(&fa, fd, STDIN_FILENO) ||
+    posix_spawn_file_actions_adddup2(&fa, fd, STDOUT_FILENO) ||
+    posix_spawn_file_actions_addclose(&fa, fd)) {
+ fprintf(stderr, _("device_script(\"%s\"): file descriptor preparation failure: %s\n"),
+ scriptname, strerror(errno));
+ goto out_destroy;
+ }
+ if (posix_spawn(&pid, scriptname, &fa, NULL, NULL, envp)) {
+ fprintf(stderr, _("device_script(\"%s\"): script execution failure: %s\n"),
+ scriptname, strerror(errno));
+ goto out_destroy;
+ }
+
+ if (pid != waitpid(pid, &status, 0) && WIFEXITED(status) && !WEXITSTATUS(status)) {
+ fprintf(stderr, _("device_script(\"%s\"): child script execution failure: %s, exit code=%d\n"), scriptname,
+ (WIFEXITED(status) ? _("normal exit") : _("abnormal exit")),
+ (WIFEXITED(status) ? WEXITSTATUS(status) : -1));
+ } else {
+ ret = 0;
+ }
+
+out_destroy:
+ posix_spawn_file_actions_destroy(&fa);
+out_free:
+ free(envp);
+out:
+ return ret;
+}
diff --git a/common/winscript.c b/common/winscript.c
new file mode 100644
index 00000000..ea9abd2b
--- /dev/null
+++ b/common/winscript.c
@@ -0,0 +1,36 @@
+/*
+
+  G N O K I I
+
+  A Linux/Unix toolset and driver for the mobile phones.
+
+  This file is part of gnokii.
+
+  Copyright (C) 2018       Ladislav Michl
+
+*/
+
+int device_script(int fd, int connect, struct gn_statemachine *state)
+{
+ /* TODO: something like this...
+ PROCESS_INFORMATION pi;
+ STARTUPINFO si;
+ ULONG rc;
+
+ ZeroMemory(&pi, sizeof(PROCESS_INFORMATION));
+ ZeroMemory(&si, sizeof(STARTUPINFO));
+
+ si.hStdOutput = fd;
+ si.hStdInput = fd;
+ si.dwFlags = STARTF_USESTDHANDLES;
+
+ if (!CreateProcess(NULL, cmdline, NULL, NULL, TRUE, 0,  NULL, NULL, &siStartInfo, &piProcInfo))
+ return -1;
+
+ WaitForSingleObject(pi.hProcess, INFINITE);
+ if(!GetExitCodeProcess(pi.hProcess, &rc))
+ rc = 0;
+ ...
+ */
+ return 0;
+}
diff --git a/configure.ac b/configure.ac
index e6cefb54..696f8f76 100644
--- a/configure.ac
+++ b/configure.ac
@@ -923,6 +923,8 @@ AC_CHECK_FUNCS(mktime timegm gettimeofday select poll wcrtomb)
 AC_CHECK_FUNCS(strchr strdup strndup strstr strtol strtok strsep)
 AC_CHECK_FUNCS(asprintf vasprintf snprintf vsnprintf getpass setenv)
 AC_CHECK_FUNCS(getaddrinfo)
+AC_CHECK_FUNCS(posix_spawn)
+AM_CONDITIONAL([HAVE_POSIX_SPAWN], [test "x$ac_cv_func_posix_spawn" = xyes])
 AC_CACHE_CHECK(for ISO C99 compliant snprintf,ac_cv_func_snprintf_c99,
  [AC_TRY_RUN([
 #include <stdio.h>
diff --git a/include/gnokii-internal.h b/include/gnokii-internal.h
index c4f6d625..9af2d8cc 100644
--- a/include/gnokii-internal.h
+++ b/include/gnokii-internal.h
@@ -20,6 +20,7 @@
 #ifndef _gnokii_internal_h
 #define _gnokii_internal_h
 
+#include "cfgreader.h"
 #include "compat.h"
 #include "misc.h"
 #if !defined(GNOKII_DEPRECATED)
@@ -154,8 +155,10 @@ int sms_nokia_text_encode(unsigned char *text, unsigned char *message, bool firs
 int sms_nokia_bitmap_encode(gn_bmp *bitmap, unsigned char *message, bool first);
 
 struct gn_cfg_header *cfg_file_read(const char *filename);
-typedef void (*cfg_foreach_func)(const char *section, const char *key, const char *value);
-void cfg_foreach(const char *section, cfg_foreach_func func);
+#define cfg_foreach_entry(section, header, entry) \
+ for (; header != NULL; header = header->next) \
+ if (strcmp(section, h->section) == 0) \
+ for (entry = h->entries; entry != NULL; entry = entry->next)
 char *cfg_set(struct gn_cfg_header *cfg, const char *section, const char *key, const char *value);
 int cfg_file_write(struct gn_cfg_header *cfg, const char *filename);
 /* Get some information about the given phone */
@@ -179,4 +182,13 @@ int strip_slashes(char *dest, const char *src, int maxlen, int len);
 /* authentication for at driver */
 gn_error do_auth(gn_auth_type auth_type, struct gn_statemachine *state);
 
+#if defined(HAVE_POSIX_SPAWN) || defined(WIN32)
+int device_script(int fd, int connect, struct gn_statemachine *state);
+#else
+static int device_script(int fd, int connect, struct gn_statemachine *state)
+{
+ return 0;
+}
+#endif
+
 #endif /* _gnokii_internal_h */
--
2.19.1


_______________________________________________
gnokii-users mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/gnokii-users