patch 8.1.0470: pointer ownership around fname_expand() is unclear

Problem:    Pointer ownership around fname_expand() is unclear.
Solution:   Allow b_ffname and b_sfname to point to the same allocated memory,
            only free one.  Update comments.

diff --git a/src/buffer.c b/src/buffer.c
index 35dd65fc9c33c774c7c1be725f5fe132a64c348c..cbccc7026c738b6e13fbc0bf20e68d3817217287 100644 (file)
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -663,8 +663,11 @@ aucmd_abort:
            workshop_file_closed_lineno((char *)buf->b_ffname,
                        (int)buf->b_last_cursor.lnum);
 #endif
-       vim_free(buf->b_ffname);
-       vim_free(buf->b_sfname);
+       if (buf->b_sfname != buf->b_ffname)
+           VIM_CLEAR(buf->b_sfname);
+       else
+           buf->b_sfname = NULL;
+       VIM_CLEAR(buf->b_ffname);
        if (buf->b_prev == NULL)
            firstbuf = buf->b_next;
        else
@@ -1877,11 +1880,13 @@ curbuf_reusable(void)
  */
     buf_T *
 buflist_new(
-    char_u     *ffname,        /* full path of fname or relative */
-    char_u     *sfname,        /* short fname or NULL */
-    linenr_T   lnum,           /* preferred cursor line */
-    int                flags)          /* BLN_ defines */
+    char_u     *ffname_arg,    // full path of fname or relative
+    char_u     *sfname_arg,    // short fname or NULL
+    linenr_T   lnum,           // preferred cursor line
+    int                flags)          // BLN_ defines
 {
+    char_u     *ffname = ffname_arg;
+    char_u     *sfname = sfname_arg;
     buf_T      *buf;
 #ifdef UNIX
     stat_T     st;
@@ -1890,7 +1895,7 @@ buflist_new(
     if (top_file_num == 1)
        hash_init(&buf_hashtab);
 
-    fname_expand(curbuf, &ffname, &sfname);    /* will allocate ffname */
+    fname_expand(curbuf, &ffname, &sfname);    // will allocate ffname
 
     /*
      * If file name already exists in the list, update the entry.
@@ -1997,8 +2002,11 @@ buflist_new(
     if ((ffname != NULL && (buf->b_ffname == NULL || buf->b_sfname == NULL))
            || buf->b_wininfo == NULL)
     {
+       if (buf->b_sfname != buf->b_ffname)
+           VIM_CLEAR(buf->b_sfname);
+       else
+           buf->b_sfname = NULL;
        VIM_CLEAR(buf->b_ffname);
-       VIM_CLEAR(buf->b_sfname);
        if (buf != curbuf)
            free_buffer(buf);
        return NULL;
@@ -3103,7 +3111,8 @@ buflist_name_nr(
 }
 
 /*
- * Set the file name for "buf"' to 'ffname', short file name to 'sfname'.
+ * Set the file name for "buf"' to "ffname_arg", short file name to
+ * "sfname_arg".
  * The file name with the full path is also remembered, for when :cd is used.
  * Returns FAIL for failure (file name already in use by other buffer)
  *     OK otherwise.
@@ -3111,10 +3120,12 @@ buflist_name_nr(
     int
 setfname(
     buf_T      *buf,
-    char_u     *ffname,
-    char_u     *sfname,
+    char_u     *ffname_arg,
+    char_u     *sfname_arg,
     int                message)        /* give message when buffer already exists */
 {
+    char_u     *ffname = ffname_arg;
+    char_u     *sfname = sfname_arg;
     buf_T      *obuf = NULL;
 #ifdef UNIX
     stat_T     st;
@@ -3123,8 +3134,11 @@ setfname(
     if (ffname == NULL || *ffname == NUL)
     {
        /* Removing the name. */
+       if (buf->b_sfname != buf->b_ffname)
+           VIM_CLEAR(buf->b_sfname);
+       else
+           buf->b_sfname = NULL;
        VIM_CLEAR(buf->b_ffname);
-       VIM_CLEAR(buf->b_sfname);
 #ifdef UNIX
        st.st_dev = (dev_T)-1;
 #endif
@@ -3175,8 +3189,9 @@ setfname(
 # endif
            fname_case(sfname, 0);    /* set correct case for short file name */
 #endif
+       if (buf->b_sfname != buf->b_ffname)
+           vim_free(buf->b_sfname);
        vim_free(buf->b_ffname);
-       vim_free(buf->b_sfname);
        buf->b_ffname = ffname;
        buf->b_sfname = sfname;
     }
@@ -3210,7 +3225,8 @@ buf_set_name(int fnum, char_u *name)
     buf = buflist_findnr(fnum);
     if (buf != NULL)
     {
-       vim_free(buf->b_sfname);
+       if (buf->b_sfname != buf->b_ffname)
+           vim_free(buf->b_sfname);
        vim_free(buf->b_ffname);
        buf->b_ffname = vim_strsave(name);
        buf->b_sfname = NULL;
@@ -4820,8 +4836,12 @@ fix_fname(char_u  *fname)
 }
 
 /*
- * Make "ffname" a full file name, set "sfname" to "ffname" if not NULL.
- * "ffname" becomes a pointer to allocated memory (or NULL).
+ * Make "*ffname" a full file name, set "*sfname" to "*ffname" if not NULL.
+ * "*ffname" becomes a pointer to allocated memory (or NULL).
+ * When resolving a link both "*sfname" and "*ffname" will point to the same
+ * allocated memory.
+ * The "*ffname" and "*sfname" pointer values on call will not be freed.
+ * Note that the resulting "*ffname" pointer should be considered not allocaed.
  */
     void
 fname_expand(
@@ -4829,18 +4849,18 @@ fname_expand(
     char_u     **ffname,
     char_u     **sfname)
 {
-    if (*ffname == NULL)       /* if no file name given, nothing to do */
+    if (*ffname == NULL)           // no file name given, nothing to do
        return;
-    if (*sfname == NULL)       /* if no short file name given, use ffname */
+    if (*sfname == NULL)           // no short file name given, use ffname
        *sfname = *ffname;
-    *ffname = fix_fname(*ffname);   /* expand to full path */
+    *ffname = fix_fname(*ffname);   // expand to full path
 
 #ifdef FEAT_SHORTCUT
     if (!buf->b_p_bin)
     {
        char_u  *rfname;
 
-       /* If the file name is a shortcut file, use the file it links to. */
+       // If the file name is a shortcut file, use the file it links to.
        rfname = mch_resolve_shortcut(*ffname);
        if (rfname != NULL)
        {

diff --git a/src/ex_cmds.c b/src/ex_cmds.c
index 0a5387c52b30eabbf6d9e9a45e1ad817f83cd6e1..7512785c9b5cc361aba76c77bc8a4422c7e83d39 100644 (file)
--- a/src/ex_cmds.c
+++ b/src/ex_cmds.c
@@ -3648,8 +3648,8 @@ check_readonly(int *forceit, buf_T *buf)
 }
 
 /*
- * Try to abandon current file and edit a new or existing file.
- * "fnum" is the number of the file, if zero use ffname/sfname.
+ * Try to abandon the current file and edit a new or existing file.
+ * "fnum" is the number of the file, if zero use "ffname_arg"/"sfname_arg".
  * "lnum" is the line number for the cursor in the new file (if non-zero).
  *
  * Return:
@@ -3661,12 +3661,14 @@ check_readonly(int *forceit, buf_T *buf)
     int
 getfile(
     int                fnum,
-    char_u     *ffname,
-    char_u     *sfname,
+    char_u     *ffname_arg,
+    char_u     *sfname_arg,
     int                setpm,
     linenr_T   lnum,
     int                forceit)
 {
+    char_u     *ffname = ffname_arg;
+    char_u     *sfname = sfname_arg;
     int                other;
     int                retval;
     char_u     *free_me = NULL;

diff --git a/src/fileio.c b/src/fileio.c
index 90f8517db173a5946dc701a9d7f09d6309447a76..4380067fd525e1862d00f79ef3c16e9b67025bc0 100644 (file)
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -6187,7 +6187,8 @@ shorten_buf_fname(buf_T *buf, char_u *dirname, int force)
                || buf->b_sfname == NULL
                || mch_isFullName(buf->b_sfname)))
     {
-       VIM_CLEAR(buf->b_sfname);
+       if (buf->b_sfname != buf->b_ffname)
+           VIM_CLEAR(buf->b_sfname);
        p = shorten_fname(buf->b_ffname, dirname);
        if (p != NULL)
        {

diff --git a/src/structs.h b/src/structs.h
index da4ffb85685901625241f41084bd64a7a9e0ac28..ad03cd4776ebfc46baca20ea420076b0bfa2ba8b 100644 (file)
--- a/src/structs.h
+++ b/src/structs.h
@@ -1972,9 +1972,11 @@ struct file_buffer
      * b_fname is the same as b_sfname, unless ":cd" has been done,
      *         then it is the same as b_ffname (NULL for no name).
      */
-    char_u     *b_ffname;      /* full path file name */
-    char_u     *b_sfname;      /* short file name */
-    char_u     *b_fname;       /* current file name */
+    char_u     *b_ffname;      // full path file name, allocated
+    char_u     *b_sfname;      // short file name, allocated, may be equal to
+                               // b_ffname
+    char_u     *b_fname;       // current file name, points to b_ffname or
+                               // b_sfname
 
 #ifdef UNIX
     int                b_dev_valid;    /* TRUE when b_dev has a valid number */

diff --git a/src/version.c b/src/version.c
index 12270b781828cf302657144f0c6f2e9dac5ebde7..3257bd2d236908f2dbf8e7ebf46da4bed03a8201 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -792,6 +792,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    470,
 /**/
     469,
 /**/