Fix two strcpy() calls in curses and needed to redefine os_read_file_name().

The os_read_file_name() function has been a source of odd looks for a
long time.  The biggest of the reasons to look at it funny was how it
passed a string back to the caller by reference without clearly stating
how big of a buffer was being written to.  It was always FILENAME_MAX,
but still, it looked like a specialized strcpy().

diff --git a/src/common/fastmem.c b/src/common/fastmem.c
index 672c308bae9ace93fc07c29824cae5ee76ae6dfe..5041210d99dd675cb579e9ea10d70ecb18e45a5a 100644 (file)
--- a/src/common/fastmem.c
+++ b/src/common/fastmem.c
@@ -663,7 +663,7 @@ static void get_default_name (char *default_name, zword addr)
  */
 void z_restore (void)
 {
-    char new_name[MAX_FILE_NAME + 1];
+    char *new_name;
     char default_name[MAX_FILE_NAME + 1];
     FILE *gfp = NULL;
 
@@ -675,7 +675,8 @@ void z_restore (void)
 
        get_default_name (default_name, (zargc >= 3) ? zargs[2] : 0);
 
-       if (os_read_file_name (new_name, default_name, FILE_LOAD_AUX) == 0)
+       new_name = os_read_file_name(default_name, FILE_LOAD_AUX);
+       if (new_name == NULL)
            goto finished;
 
        free(f_setup.aux_name);
@@ -703,7 +704,8 @@ void z_restore (void)
 
        /* Get the file name */
 
-       if (os_read_file_name (new_name, f_setup.save_name, FILE_RESTORE) == 0)
+       new_name = os_read_file_name(f_setup.save_name, FILE_RESTORE);
+       if (new_name == NULL)
            goto finished;
 
        free(f_setup.save_name);
@@ -903,7 +905,7 @@ void z_restore_undo (void)
  */
 void z_save (void)
 {
-    char new_name[MAX_FILE_NAME + 1];
+    char *new_name;
     char default_name[MAX_FILE_NAME + 1];
     FILE *gfp;
 
@@ -915,7 +917,8 @@ void z_save (void)
 
        get_default_name (default_name, (zargc >= 3) ? zargs[2] : 0);
 
-       if (os_read_file_name (new_name, default_name, FILE_SAVE_AUX) == 0)
+       new_name = os_read_file_name(default_name, FILE_SAVE_AUX);
+       if (new_name == NULL)
            goto finished;
 
        free(f_setup.aux_name);
@@ -944,7 +947,8 @@ void z_save (void)
 
        /* Get the file name */
 
-       if (os_read_file_name (new_name, f_setup.save_name, FILE_SAVE) == 0)
+       new_name = os_read_file_name(f_setup.save_name, FILE_SAVE);
+       if (new_name == NULL)
            goto finished;
 
        free(f_setup.save_name);

diff --git a/src/common/files.c b/src/common/files.c
index cd54c154000561e8c6ed364b3136cc58ea575832..676c0ddfb18e3a84c1ce178d5249bb8c8db2534d 100644 (file)
--- a/src/common/files.c
+++ b/src/common/files.c
@@ -66,13 +66,14 @@ void script_open (void)
 {
     static bool script_valid = FALSE;
 
-    char new_name[MAX_FILE_NAME + 1];
+    char *new_name;
 
     h_flags &= ~SCRIPTING_FLAG;
 
     if (h_version >= V5 || !script_valid) {
 
-       if (!os_read_file_name (new_name, f_setup.script_name, FILE_SCRIPT))
+       new_name = os_read_file_name(f_setup.script_name, FILE_SCRIPT);
+       if (new_name == NULL)
            goto done;
 
        free(f_setup.script_name);
@@ -291,9 +292,10 @@ void script_mssg_off (void)
 
 void record_open (void)
 {
-    char new_name[MAX_FILE_NAME + 1];
+    char *new_name;
 
-    if (os_read_file_name (new_name, f_setup.command_name, FILE_RECORD)) {
+    new_name = os_read_file_name(f_setup.command_name, FILE_RECORD);
+    if (new_name != NULL) {
 
        free(f_setup.command_name);
        f_setup.command_name = strdup(new_name);
@@ -416,9 +418,10 @@ void record_write_input (const zchar *buf, zchar key)
 
 void replay_open (void)
 {
-    char new_name[MAX_FILE_NAME + 1];
+    char *new_name;
 
-    if (os_read_file_name (new_name, f_setup.command_name, FILE_PLAYBACK)) {
+    new_name = os_read_file_name(f_setup.command_name, FILE_PLAYBACK);
+    if (new_name != NULL) {
 
        free(f_setup.command_name);
        f_setup.command_name = strdup(new_name);

diff --git a/src/common/frotz.h b/src/common/frotz.h
index 682d1aed579698b05df134de2a11834d8022458b..bf71fe0c369f6e11a2d0b40122d038151afc1d4b 100644 (file)
--- a/src/common/frotz.h
+++ b/src/common/frotz.h
@@ -813,7 +813,7 @@ int         os_picture_data (int, int *, int *);
 void   os_prepare_sample (int);
 void   os_process_arguments (int, char *[]);
 int    os_random_seed (void);
-int    os_read_file_name (char *, const char *, int);
+char   *os_read_file_name (const char *, int);
 zchar  os_read_key (int, int);
 zchar  os_read_line (int, zchar *, int, int, int);
 void   os_reset_screen (void);

diff --git a/src/curses/ux_input.c b/src/curses/ux_input.c
index 1181f5d2d339209528355ac678ed3e12aa7211ad..542ffd0bbf5767d5f89b819818e0d3e84a8bc959 100644 (file)
--- a/src/curses/ux_input.c
+++ b/src/curses/ux_input.c
@@ -711,7 +711,7 @@ zchar os_read_key (int timeout, int cursor)
 /*
  * os_read_file_name
  *
- * Return the name of a file. Flag can be one of:
+ * Return the name of a file.  Flag can be one of:
  *
  *    FILE_SAVE     - Save game file
  *    FILE_RESTORE  - Restore game file
@@ -726,9 +726,10 @@ zchar os_read_key (int timeout, int cursor)
  * name. If it is unable to do that then this function should call
  * print_string and read_string to ask for a file name.
  *
+ * Return value is NULL if there was a problem.
  */
 
-int os_read_file_name (char *file_name, const char *default_name, int flag)
+char *os_read_file_name (const char *default_name, int flag)
 {
     FILE *fp;
     int saved_replay = istream_replay;
@@ -737,6 +738,7 @@ int os_read_file_name (char *file_name, const char *default_name, int flag)
     char *tempname;
     zchar answer[4];
     char path_separator[2];
+    char file_name[FILENAME_MAX + 1];
 
     /* Turn off playback and recording temporarily */
     istream_replay = 0;
@@ -772,12 +774,12 @@ int os_read_file_name (char *file_name, const char *default_name, int flag)
     if (f_setup.restricted_path) {
        tempname = dirname(file_name);
        if (strlen(tempname) > 1)
-           return 0;
+           return NULL;
     }
 
     /* Use the default name if nothing was typed */
     if (file_name[0] == 0)
-        strcpy (file_name, default_name);
+        strncpy (file_name, default_name, FILENAME_MAX);
 
     /* If we're restricted to one directory, strip any leading path left
      * over from a previous call to os_read_file_name(), then prepend
@@ -796,11 +798,11 @@ int os_read_file_name (char *file_name, const char *default_name, int flag)
            }
        }
        tempname = strdup(file_name + i);
-       strcpy(file_name, f_setup.restricted_path);
+       strncpy(file_name, f_setup.restricted_path, FILENAME_MAX);
 
        /* Make sure the final character is the path separator. */
        if (file_name[strlen(file_name)-1] != PATH_SEPARATOR) {
-           strncat(file_name, path_separator, strlen(file_name) - 2);
+           strncat(file_name, path_separator, FILENAME_MAX - strlen(file_name) - 2);
        }
        strncat(file_name, tempname, strlen(file_name) - strlen(tempname) - 1);
     }
@@ -811,14 +813,15 @@ int os_read_file_name (char *file_name, const char *default_name, int flag)
        fclose (fp);
        print_string("Overwrite existing file? ");
        read_string(4, answer);
-       return(tolower(answer[0]) == 'y');
+       if (tolower(answer[0] != 'y'))
+           return NULL;
     }
 
     /* Restore state of playback and recording */
     istream_replay = saved_replay;
     ostream_record = saved_record;
 
-    return 1;
+    return strdup(file_name);
 
 } /* os_read_file_name */
 

diff --git a/src/dumb/dumb_input.c b/src/dumb/dumb_input.c
index 805b3f65474d98b46f5f10757b5598c8e7bd6f3e..03058ad6ff2a35c7820e9b0f278b1483431e9370 100644 (file)
--- a/src/dumb/dumb_input.c
+++ b/src/dumb/dumb_input.c
@@ -409,8 +409,9 @@ zchar os_read_line (int UNUSED (max), zchar *buf, int timeout, int UNUSED(width)
   return terminator;
 }
 
-int os_read_file_name (char *file_name, const char *default_name, int flag)
+char *os_read_file_name (const char *default_name, int flag)
 {
+  char file_name[FILENAME_MAX + 1];
   char buf[INPUT_BUFFER_SIZE], prompt[INPUT_BUFFER_SIZE];
   FILE *fp;
   char *tempname;
@@ -420,8 +421,7 @@ int os_read_file_name (char *file_name, const char *default_name, int flag)
    * our filename is already provided.  Just go ahead silently.
    */
   if (f_setup.restore_mode) {
-    strcpy(file_name, default_name);
-    return TRUE;
+    return strdup(default_name);
   } else {
     if (f_setup.restricted_path) {
       for (i = strlen(default_name); i > 0; i--) {
@@ -437,7 +437,7 @@ int os_read_file_name (char *file_name, const char *default_name, int flag)
     dumb_read_misc_line(buf, prompt);
     if (strlen(buf) > MAX_FILE_NAME) {
       printf("Filename too long\n");
-      return FALSE;
+      return NULL;
     }
   }
 
@@ -464,9 +464,10 @@ int os_read_file_name (char *file_name, const char *default_name, int flag)
       && ((fp = fopen(file_name, "rb")) != NULL)) {
     fclose (fp);
     dumb_read_misc_line(buf, "Overwrite existing file? ");
-    return(tolower(buf[0]) == 'y');
+    if (tolower(buf[0]) != 'y')
+       return NULL;
   }
-  return TRUE;
+  return strdup(file_name);
 }
 
 void os_more_prompt (void)

diff --git a/src/sdl/sf_util.c b/src/sdl/sf_util.c
index fbd8e1e2da0e5ddb312427c050416030e74c2404..68041442537af7a697cc7212bb22bd1db5bd0e5e 100644 (file)
--- a/src/sdl/sf_util.c
+++ b/src/sdl/sf_util.c
@@ -444,25 +444,6 @@ unsigned long sf_ticks (void) {
 
 #endif
 
-/*
- * os_read_file_name
- *
- * Return the name of a file. Flag can be one of:
- *
- *    FILE_SAVE     - Save game file
- *    FILE_RESTORE  - Restore game file
- *    FILE_SCRIPT   - Transscript file
- *    FILE_RECORD   - Command file for recording
- *    FILE_PLAYBACK - Command file for playback
- *    FILE_SAVE_AUX - Save auxilary ("preferred settings") file
- *    FILE_LOAD_AUX - Load auxilary ("preferred settings") file
- *
- * The length of the file name is limited by MAX_FILE_NAME. Ideally
- * an interpreter should open a file requester to ask for the file
- * name. If it is unable to do that then this function should call
- * print_string and read_string to ask for a file name.
- *
- */
 
 static char *getextension( int flag)
   {
@@ -520,10 +501,33 @@ static const char *getdatename( const char *def, char *ext)
 static int ingame_read_file_name (char *file_name, const char *default_name, int flag);
 static int dialog_read_file_name (char *file_name, const char *default_name, int flag);
 
-int os_read_file_name (char *file_name, const char *default_name, int flag)
+
+/*
+ * os_read_file_name
+ *
+ * Return the name of a file. Flag can be one of:
+ *
+ *    FILE_SAVE     - Save game file
+ *    FILE_RESTORE  - Restore game file
+ *    FILE_SCRIPT   - Transscript file
+ *    FILE_RECORD   - Command file for recording
+ *    FILE_PLAYBACK - Command file for playback
+ *    FILE_SAVE_AUX - Save auxilary ("preferred settings") file
+ *    FILE_LOAD_AUX - Load auxilary ("preferred settings") file
+ *
+ * The length of the file name is limited by MAX_FILE_NAME. Ideally
+ * an interpreter should open a file requester to ask for the file
+ * name. If it is unable to do that then this function should call
+ * print_string and read_string to ask for a file name.
+ *
+ * Return value is NULL is there was a problem
+ */
+
+char *os_read_file_name (const char *default_name, int flag)
   {
   int st;
   const char *initname = default_name;
+  char file_name[FILENAME_MAX + 1];
 
   if (newfile(flag))
     {
@@ -533,8 +537,12 @@ int os_read_file_name (char *file_name, const char *default_name, int flag)
     }
 
   st = dialog_read_file_name( file_name, initname, flag);
-  if (st == SF_NOTIMP) st = ingame_read_file_name( file_name, initname, flag);
-  return st;
+  if (st == SF_NOTIMP)
+    st = ingame_read_file_name( file_name, initname, flag);
+
+  if (!st) return NULL;
+
+  return strdup(file_name);
   }
 
 static int ingame_read_file_name (char *file_name, const char *default_name, int flag)