Split g_file_open() into _ro() and _rw() variants

Rename g_file_open() to g_file_open_rw(), and add a new g_file_open_ro()
call that wraps the common g_file_open_ex(file, 1, 0, 0, 0) idiom. This
will make the file access mode more explicit in the code.

Change all calls to g_file_open() to the _ro() or _rw() variant as
appropriate, and replace g_file_open_ex(file, 1, 0, 0, 0) with the _ro()
call.

Lastly, add tests for the two new calls to test_os_calls.c (code
courteously provided by matt335672).

common/file.c

@@ -349,7 +349,7 @@ file_by_name_read_sections(const char *file_name, struct list *names)
         return 1;
     }
 
-    fd = g_file_open_ex(file_name, 1, 0, 0, 0);
+    fd = g_file_open_ro(file_name);
 
     if (fd < 0)
     {
@@ -390,7 +390,7 @@ file_by_name_read_section(const char *file_name, const char *section,
         return 1;
     }
 
-    fd = g_file_open_ex(file_name, 1, 0, 0, 0);
+    fd = g_file_open_ro(file_name);
 
     if (fd < 0)
     {

common/log.c

@@ -689,7 +689,7 @@ log_config_init_from_config(const char *iniFilename,
         return NULL;
     }
 
-    fd = g_file_open_ex(iniFilename, 1, 0, 0, 0);
+    fd = g_file_open_ro(iniFilename);
 
     if (-1 == fd)
     {

common/os_calls.c

@@ -2086,24 +2086,14 @@ g_memcmp(const void *s1, const void *s2, int len)
 /*****************************************************************************/
 /* returns -1 on error, else return handle or file descriptor */
 int
-g_file_open(const char *file_name)
+g_file_open_rw(const char *file_name)
 {
 #if defined(_WIN32)
     return (int)CreateFileA(file_name, GENERIC_READ | GENERIC_WRITE,
                             FILE_SHARE_READ | FILE_SHARE_WRITE,
                             0, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, 0);
 #else
-    int rv;
+    return open(file_name, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
-
-    rv =  open(file_name, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
-
-    if (rv == -1)
-    {
-        /* can't open read / write, try to open read only */
-        rv =  open(file_name, O_RDONLY);
-    }
-
-    return rv;
 #endif
 }
 
@@ -2145,6 +2135,14 @@ g_file_open_ex(const char *file_name, int aread, int awrite,
 #endif
 }
 
+/*****************************************************************************/
+/* returns -1 on error, else return handle or file descriptor */
+int
+g_file_open_ro(const char *file_name)
+{
+    return g_file_open_ex(file_name, 1, 0, 0, 0);
+}
+
 /*****************************************************************************/
 /* returns error, always 0 */
 int

common/os_calls.h

@@ -206,9 +206,10 @@ int      g_obj_wait(tintptr *read_objs, int rcount, tintptr *write_objs,
 void     g_random(char *data, int len);
 int      g_abs(int i);
 int      g_memcmp(const void *s1, const void *s2, int len);
-int      g_file_open(const char *file_name);
+int      g_file_open_rw(const char *file_name);
 int      g_file_open_ex(const char *file_name, int aread, int awrite,
                         int acreate, int atrunc);
+int      g_file_open_ro(const char *file_name);
 int      g_file_close(int fd);
 /**
  * Returns 1 if a file is open (i.e. the file descriptor is valid)

fontutils/fv1.c

@@ -170,7 +170,7 @@ fv1_file_load(const char *filename)
             int fd;
             make_stream(s);
             init_stream(s, file_size + 1024);
-            fd = g_file_open(filename);
+            fd = g_file_open_ro(filename);
 
             if (fd < 0)
             {

fontutils/windows/fontdump.c

@@ -155,7 +155,7 @@ font_dump(void)
     g_snprintf(filename, 255, "%s-%d.fv1", g_font_name, g_font_size);
     msg("creating file %s", filename);
     g_file_delete(filename);
-    fd = g_file_open(filename);
+    fd = g_file_open_rw(filename);
     g_file_write(fd, "FNT1", 4);
     strlen1 = g_strlen(g_font_name);
     g_file_write(fd, g_font_name, strlen1);

keygen/keygen.c

@@ -367,7 +367,7 @@ save_all(const char *e_data, int e_len, const char *n_data, int n_len,
         }
     }
 
-    fd = g_file_open(filename);
+    fd = g_file_open_rw(filename);
 
     if (fd != -1)
     {

libxrdp/xrdp_sec.c

@@ -420,7 +420,7 @@ xrdp_load_keyboard_layout(struct xrdp_client_info *client_info)
     g_snprintf(keyboard_cfg_file, 255, "%s/xrdp_keyboard.ini", XRDP_CFG_PATH);
     LOG(LOG_LEVEL_DEBUG, "keyboard_cfg_file %s", keyboard_cfg_file);
 
-    fd = g_file_open(keyboard_cfg_file);
+    fd = g_file_open_ro(keyboard_cfg_file);
 
     if (fd >= 0)
     {

sesman/chansrv/chansrv_config.c

@@ -231,7 +231,7 @@ config_read(int use_logger, const char *sesman_ini)
     log_func_t logmsg = (use_logger) ? log_message : log_to_stdout;
     int fd;
 
-    fd = g_file_open_ex(sesman_ini, 1, 0, 0, 0);
+    fd = g_file_open_ro(sesman_ini);
     if (fd < 0)
     {
         logmsg(LOG_LEVEL_ERROR, "Can't open config file %s", sesman_ini);

sesman/chansrv/clipboard_file.c

@@ -462,7 +462,7 @@ clipboard_send_file_data(int streamId, int lindex,
               "nPositionLow %d cbRequested %d", streamId, lindex,
               nPositionLow, cbRequested);
     g_snprintf(full_fn, 255, "%s/%s", cfi->pathname, cfi->filename);
-    fd = g_file_open_ex(full_fn, 1, 0, 0, 0);
+    fd = g_file_open_ro(full_fn);
     if (fd == -1)
     {
         LOG(LOG_LEVEL_ERROR, "clipboard_send_file_data: file open [%s] failed: %s",

sesman/libsesman/sesman_config.c

@@ -583,7 +583,7 @@ config_read(const char *sesman_ini)
         if ((cfg->sesman_ini = g_strdup(sesman_ini)) != NULL)
         {
             int fd;
-            if ((fd = g_file_open_ex(cfg->sesman_ini, 1, 0, 0, 0)) != -1)
+            if ((fd = g_file_open_ro(cfg->sesman_ini)) != -1)
             {
                 struct list *sec;
                 struct list *param_n;

sesman/lock_uds.c

@@ -87,7 +87,7 @@ lock_uds(const char *sockname)
             *p++ = '\0';
 
             saved_umask = g_umask_hex(0x77);
-            fd = g_file_open(filename);
+            fd = g_file_open_rw(filename);
             g_umask_hex(saved_umask);
             if (fd < 0)
             {

sesman/sesman.c

@@ -658,7 +658,7 @@ read_pid_file(const char *pid_file, int *pid)
     {
         g_printf("sesman is not running (pid file not found - %s)\n", pid_file);
     }
-    else if ((fd = g_file_open(pid_file)) < 0)
+    else if ((fd = g_file_open_ro(pid_file)) < 0)
     {
         g_printf("error opening pid file[%s]: %s\n", pid_file, g_get_strerror());
     }
@@ -838,15 +838,15 @@ main(int argc, char **argv)
         g_file_close(1);
         g_file_close(2);
 
-        if (g_file_open("/dev/null") < 0)
+        if (g_file_open_rw("/dev/null") < 0)
         {
         }
 
-        if (g_file_open("/dev/null") < 0)
+        if (g_file_open_rw("/dev/null") < 0)
         {
         }
 
-        if (g_file_open("/dev/null") < 0)
+        if (g_file_open_rw("/dev/null") < 0)
         {
         }
     }
@@ -905,7 +905,7 @@ main(int argc, char **argv)
     {
         /* writing pid file */
         char pid_s[32];
-        int fd = g_file_open(pid_file);
+        int fd = g_file_open_rw(pid_file);
 
         if (-1 == fd)
         {

tests/common/test_os_calls.c

@@ -17,6 +17,9 @@
 #define TOP_SRCDIR "."
 #endif
 
+// File for testing ro/rw opens
+#define RO_RW_FILE "./test_ro_rw"
+
 /******************************************************************************/
 /***
  * Gets the number of open file descriptors for the current process */
@@ -199,12 +202,51 @@ START_TEST(test_g_file_get_size__5GiB)
 END_TEST
 #endif
 
+/******************************************************************************/
+/* Test we can write to a file which is opened for write */
+START_TEST(test_g_file_rw)
+{
+    const char data[] = "File data\n";
+
+    int fd = g_file_open_rw(RO_RW_FILE);
+    ck_assert(fd >= 0);
+
+    int status = g_file_write(fd, data, sizeof(data) - 1);
+    g_file_close(fd);
+
+    // Assume no signals have occurred
+    ck_assert_int_eq(status, sizeof(data) - 1);
+
+    // Leave file in place for test_g_file_ro
+}
+END_TEST
+
+/******************************************************************************/
+/* Test we can't write to a file which is opened read only */
+START_TEST(test_g_file_ro)
+{
+    const char data[] = "File data\n";
+
+    int fd = g_file_open_ro(RO_RW_FILE);
+    ck_assert(fd >= 0);
+
+    int status = g_file_write(fd, data, sizeof(data) - 1);
+    g_file_close(fd);
+
+    // Write must fail
+    ck_assert_int_lt(status, 0);
+
+    // Tidy-up (not checked)
+    g_file_delete(RO_RW_FILE);
+}
+END_TEST
+
 /******************************************************************************/
 /* Just test we can set and clear the flag. We don't test its operation */
 START_TEST(test_g_file_cloexec)
 {
     int flag;
-    int devzerofd = g_file_open("/dev/zero");
+    int devzerofd = g_file_open_ro("/dev/zero");
     ck_assert(devzerofd >= 0);
 
     (void)g_file_set_cloexec(devzerofd, 1);
@@ -230,7 +272,7 @@ START_TEST(test_g_file_get_open_fds)
     ck_assert_int_eq(start_list->count, fd_count);
 
     // Open another file
-    int devzerofd = g_file_open("/dev/zero");
+    int devzerofd = g_file_open_ro("/dev/zero");
     ck_assert(devzerofd >= 0);
 
     // Have we now got one more open file?
@@ -266,7 +308,7 @@ END_TEST
 /******************************************************************************/
 START_TEST(test_g_file_is_open)
 {
-    int devzerofd = g_file_open("/dev/zero");
+    int devzerofd = g_file_open_ro("/dev/zero");
     ck_assert(devzerofd >= 0);
 
     // Check open file comes up as open
@@ -287,7 +329,7 @@ START_TEST(test_g_sck_fd_passing)
     int istatus;
     unsigned int fdcount;
 
-    int devzerofd = g_file_open("/dev/zero");
+    int devzerofd = g_file_open_ro("/dev/zero");
     ck_assert(devzerofd >= 0);
 
     if (g_sck_local_socketpair(sck) != 0)
@@ -369,8 +411,8 @@ START_TEST(test_g_sck_fd_overflow)
     unsigned int proc_fd_count;
 
     // Open a couple of file descriptors to /dev/zero
-    devzerofd[0] = g_file_open("/dev/zero");
+    devzerofd[0] = g_file_open_ro("/dev/zero");
-    devzerofd[1] = g_file_open("/dev/zero");
+    devzerofd[1] = g_file_open_ro("/dev/zero");
     ck_assert(devzerofd[0] >= 0);
     ck_assert(devzerofd[1] >= 0);
     proc_fd_count = get_open_fd_count();
@@ -463,6 +505,8 @@ make_suite_test_os_calls(void)
     tcase_add_test(tc_os_calls, test_g_file_get_size__2GiB);
     tcase_add_test(tc_os_calls, test_g_file_get_size__5GiB);
 #endif
+    tcase_add_test(tc_os_calls, test_g_file_rw);
+    tcase_add_test(tc_os_calls, test_g_file_ro); // Must follow test_g_file_rw
     tcase_add_test(tc_os_calls, test_g_file_cloexec);
     tcase_add_test(tc_os_calls, test_g_file_get_open_fds);
     tcase_add_test(tc_os_calls, test_g_file_is_open);

tests/libipm/test_libipm_main.c

@@ -58,7 +58,7 @@ suite_test_libipm_calls_start(void)
         const char *errstr = g_get_strerror();
         LOG(LOG_LEVEL_ERROR, "Can't create test transport 3 [%s]", errstr);
     }
-    else if ((fd = g_file_open("/dev/zero")) < 0)
+    else if ((fd = g_file_open_rw("/dev/zero")) < 0)
     {
         const char *errstr = g_get_strerror();
         LOG(LOG_LEVEL_ERROR, "Can't open /dev/zero [%s]", errstr);

xrdp/lang.c

@@ -288,7 +288,7 @@ int km_load_file(const char *filename, struct xrdp_keymap *keymap)
     int fd;
 
     LOG(LOG_LEVEL_INFO, "Loading keymap file %s", filename);
-    fd = g_file_open(filename);
+    fd = g_file_open_ro(filename);
 
     if (fd != -1)
     {

xrdp/xrdp.c

@@ -375,7 +375,7 @@ main(int argc, char **argv)
 
         if (g_file_exist(pid_file)) /* xrdp.pid */
         {
-            fd = g_file_open(pid_file); /* xrdp.pid */
+            fd = g_file_open_ro(pid_file); /* xrdp.pid */
         }
 
         if (fd == -1)
@@ -450,7 +450,7 @@ main(int argc, char **argv)
         g_create_path(pid_file);
 
         /* make sure we can write to pid file */
-        fd = g_file_open(pid_file); /* xrdp.pid */
+        fd = g_file_open_rw(pid_file); /* xrdp.pid */
 
         if (fd == -1)
         {
@@ -509,7 +509,7 @@ main(int argc, char **argv)
         g_sleep(1000);
         /* write the pid to file */
         pid = g_getpid();
-        fd = g_file_open(pid_file); /* xrdp.pid */
+        fd = g_file_open_rw(pid_file); /* xrdp.pid */
 
         if (fd == -1)
         {
@@ -528,15 +528,15 @@ main(int argc, char **argv)
         g_file_close(1);
         g_file_close(2);
 
-        if (g_file_open("/dev/null") < 0)
+        if (g_file_open_rw("/dev/null") < 0)
         {
         }
 
-        if (g_file_open("/dev/null") < 0)
+        if (g_file_open_rw("/dev/null") < 0)
         {
         }
 
-        if (g_file_open("/dev/null") < 0)
+        if (g_file_open_rw("/dev/null") < 0)
         {
         }
 

xrdp/xrdp_bitmap_load.c

@@ -542,7 +542,7 @@ xrdp_bitmap_load_bmp(struct xrdp_bitmap *self, const char *filename,
         return 1;
     }
 
-    fd = g_file_open(filename);
+    fd = g_file_open_ro(filename);
 
     if (fd == -1)
     {

xrdp/xrdp_font.c

@@ -210,7 +210,7 @@ xrdp_font_create(struct xrdp_wm *wm, unsigned int dpi)
     self->wm = wm;
     make_stream(s);
     init_stream(s, file_size + 1024);
-    fd = g_file_open(file_path);
+    fd = g_file_open_ro(file_path);
 
     if (fd != -1)
     {

xrdp/xrdp_listen.c

@@ -170,7 +170,7 @@ xrdp_listen_get_startup_params(struct xrdp_listen *self)
     startup_params = self->startup_params;
     port_override = startup_params->port[0] != 0;
     fork_override = startup_params->fork;
-    fd = g_file_open(startup_params->xrdp_ini);
+    fd = g_file_open_ro(startup_params->xrdp_ini);
     if (fd != -1)
     {
         names = list_create();

xrdp/xrdp_login_wnd.c

@@ -607,7 +607,7 @@ xrdp_wm_login_fill_in_combo(struct xrdp_wm *self, struct xrdp_bitmap *b)
     section_names->auto_free = 1;
     section_values = list_create();
     section_values->auto_free = 1;
-    fd = g_file_open(xrdp_ini);
+    fd = g_file_open_ro(xrdp_ini);
 
     if (fd < 0)
     {
@@ -1107,7 +1107,7 @@ load_xrdp_config(struct xrdp_config *config, const char *xrdp_ini, int bpp)
     globals->ls_unscaled.help_wnd_height = DEFAULT_WND_HELP_H;
 
     /* open xrdp.ini file */
-    if ((fd = g_file_open(xrdp_ini)) < 0)
+    if ((fd = g_file_open_ro(xrdp_ini)) < 0)
     {
         LOG(LOG_LEVEL_ERROR, "load_config: Could not read "
             "xrdp.ini file %s", xrdp_ini);

xrdp/xrdp_mm.c

@@ -2073,7 +2073,7 @@ xrdp_mm_get_sesman_port(char *port, int port_bytes)
     g_strncpy(port, "3350", port_bytes - 1);
     /* see if port is in sesman.ini file */
     g_snprintf(cfg_file, 255, "%s/sesman.ini", XRDP_CFG_PATH);
-    fd = g_file_open(cfg_file);
+    fd = g_file_open_ro(cfg_file);
 
     if (fd >= 0)
     {
@@ -2861,7 +2861,7 @@ xrdp_mm_dump_jpeg(struct xrdp_mm *self, XRDP_ENC_DATA_DONE *enc_done)
     header.bytes_follow = enc_done->comp_bytes - (2 + pheader_bytes[0]);
     if (ii == 0)
     {
-        ii = g_file_open("/tmp/jpeg.beef.bin");
+        ii = g_file_open_rw("/tmp/jpeg.beef.bin");
         if (ii == -1)
         {
             ii = 0;

xrdp/xrdp_wm.c

@@ -245,7 +245,7 @@ xrdp_wm_load_pointer(struct xrdp_wm *self, char *file_name, char *data,
 
     make_stream(fs);
     init_stream(fs, 8192);
-    fd = g_file_open(file_name);
+    fd = g_file_open_ro(file_name);
 
     if (fd < 0)
     {
@@ -414,7 +414,7 @@ xrdp_wm_load_static_colors_plus(struct xrdp_wm *self, char *autorun_name)
     self->background = HCOLOR(self->screen->bpp, 0x000000);
 
     /* now load them from the globals in xrdp.ini if defined */
-    fd = g_file_open(self->session->xrdp_ini);
+    fd = g_file_open_ro(self->session->xrdp_ini);
 
     if (fd >= 0)
     {
@@ -638,7 +638,7 @@ xrdp_wm_init(struct xrdp_wm *self)
          * NOTE: this should eventually be accessed from self->xrdp_config
          */
 
-        fd = g_file_open(self->session->xrdp_ini);
+        fd = g_file_open_ro(self->session->xrdp_ini);
         if (fd != -1)
         {
             names = list_create();

xrdp/xrdpwin.c

@@ -214,7 +214,7 @@ MyServiceMain(DWORD dwArgc, LPTSTR *lpszArgv)
     //  int fd;
     //  char text[256];
 
-    //  fd = g_file_open("c:\\temp\\xrdp\\log.txt");
+    //  fd = g_file_open_rw("c:\\temp\\xrdp\\log.txt");
     //  g_file_write(fd, "hi\r\n", 4);
     //event_han = RegisterEventSource(0, "xrdp");
     //log_event(event_han, "hi xrdp log");
@@ -452,7 +452,7 @@ main(int argc, char **argv)
 
             if (g_file_exist(pid_file)) /* xrdp.pid */
             {
-                fd = g_file_open(pid_file); /* xrdp.pid */
+                fd = g_file_open_ro(pid_file); /* xrdp.pid */
             }
 
             if (fd == -1)
@@ -539,7 +539,7 @@ main(int argc, char **argv)
     if (!no_daemon)
     {
         /* make sure we can write to pid file */
-        fd = g_file_open(pid_file); /* xrdp.pid */
+        fd = g_file_open_rw(pid_file); /* xrdp.pid */
 
         if (fd == -1)
         {
@@ -579,9 +579,9 @@ main(int argc, char **argv)
         g_file_close(0);
         g_file_close(1);
         g_file_close(2);
-        g_file_open("/dev/null");
+        g_file_open_rw("/dev/null");
-        g_file_open("/dev/null");
+        g_file_open_rw("/dev/null");
-        g_file_open("/dev/null");
+        g_file_open_rw("/dev/null");
         /* end of daemonizing code */
     }
 
@@ -589,7 +589,7 @@ main(int argc, char **argv)
     {
         /* write the pid to file */
         pid = g_getpid();
-        fd = g_file_open(pid_file); /* xrdp.pid */
+        fd = g_file_open_rw(pid_file); /* xrdp.pid */
 
         if (fd == -1)
         {