Made reading/writing undo info a bit more robust.

runtime/doc/todo.txt

@@ -123,9 +123,6 @@ Slow combination of folding and PHP syntax highlighting.  Script to reproduce
 it.  Caused by "syntax sync fromstart" in combination with patch 7.2.274.
 (Christian Brabandt, 2010 May 27)
 
-Check for unused functions, idea:
-http://blog.flameeyes.eu/2008/01/17/today-how-to-identify-unused-exported-functions-and-variables
-
 In command line window ":close" doesn't work properly. (Tony Mechelynck, 2009
 Jun 1)
 

runtime/doc/undo.txt

@@ -282,10 +282,15 @@ Reading an existing undo file may fail for several reasons:
 	the undo file cannot be used, it would corrupt the text.  This also
 	happens when 'encoding' differs from when the undo file was written.
 *E825*  The undo file does not contain valid contents and cannot be used.
+"Not reading undo file, owner differs"
+	The undo file is owned by someone else than the owner of the text
+	file.  For safety the undo file is not used.
 
 Writing an undo file may fail for these reasons:
 *E828*	The file to be written cannot be created.  Perhaps you do not have
 	write permissions in the directory.
+"Cannot write undo file in any directory in 'undodir'"
+	None of the directories in 'undodir' can be used.
 "Will not overwrite with undo file, cannot read"
 	A file exists with the name of the undo file to be written, but it
 	cannot be read.  You may want to delete this file or rename it.
@@ -293,6 +298,9 @@ Writing an undo file may fail for these reasons:
 	A file exists with the name of the undo file to be written, but it
 	does not start with the right magic number.  You may want to delete
 	this file or rename it.
+"Skipping undo file write, noting to undo"
+	There is no undo information not be written, nothing has been changed
+	or 'undolevels' is negative.
 *E829*	An error occurred while writing the undo file.  You may want to try
 	again.
 

src/ex_docmd.c

@@ -8478,7 +8478,7 @@ ex_rundo(eap)
     char_u hash[UNDO_HASH_SIZE];
 
     u_compute_hash(hash);
-    u_read_undo(eap->arg, hash);
+    u_read_undo(eap->arg, hash, NULL);
 }
 #endif
 

src/fileio.c

@@ -2588,7 +2588,7 @@ failed:
 	char_u	hash[UNDO_HASH_SIZE];
 
 	sha256_finish(&sha_ctx, hash);
-	u_read_undo(NULL, hash);
+	u_read_undo(NULL, hash, fname);
     }
 #endif
 

src/proto/undo.pro

@@ -9,7 +9,7 @@ int undo_allowed __ARGS((void));
 void u_compute_hash __ARGS((char_u *hash));
 char_u *u_get_undo_file_name __ARGS((char_u *buf_ffname, int reading));
 void u_write_undo __ARGS((char_u *name, int forceit, buf_T *buf, char_u *hash));
-void u_read_undo __ARGS((char_u *name, char_u *hash));
+void u_read_undo __ARGS((char_u *name, char_u *hash, char_u *orig_name));
 void u_undo __ARGS((int count));
 void u_redo __ARGS((int count));
 void undo_time __ARGS((long step, int sec, int absolute));

src/undo.c

@@ -102,6 +102,9 @@ static void u_freeentry __ARGS((u_entry_T *, long));
 #ifdef FEAT_PERSISTENT_UNDO
 static void corruption_error __ARGS((char *msg, char_u *file_name));
 static void u_free_uhp __ARGS((u_header_T *uhp));
+static int serialize_header __ARGS((FILE *fp, buf_T *buf, char_u *hash));
+static int serialize_uhp __ARGS((FILE *fp, u_header_T *uhp));
+static u_header_T *unserialize_uhp __ARGS((FILE *fp, char_u *file_name));
 static int serialize_uep __ARGS((u_entry_T *uep, FILE *fp));
 static u_entry_T *unserialize_uep __ARGS((FILE *fp, int *error, char_u *file_name));
 static void serialize_pos __ARGS((pos_T pos, FILE *fp));
@@ -797,6 +800,173 @@ u_free_uhp(uhp)
     vim_free(uhp);
 }
 
+    static int
+serialize_header(fp, buf, hash)
+    FILE	*fp;
+    buf_T	*buf;
+    char_u	*hash;
+{
+    int len;
+
+    /* Start writing, first the magic marker and undo info version. */
+    if (fwrite(UF_START_MAGIC, (size_t)UF_START_MAGIC_LEN, (size_t)1, fp) != 1)
+	return FAIL;
+    put_bytes(fp, (long_u)UF_VERSION, 2);
+
+    /* Write a hash of the buffer text, so that we can verify it is still the
+     * same when reading the buffer text. */
+    if (fwrite(hash, (size_t)UNDO_HASH_SIZE, (size_t)1, fp) != 1)
+	return FAIL;
+
+    /* buffer-specific data */
+    put_bytes(fp, (long_u)buf->b_ml.ml_line_count, 4);
+    len = buf->b_u_line_ptr != NULL ? (int)STRLEN(buf->b_u_line_ptr) : 0;
+    put_bytes(fp, (long_u)len, 4);
+    if (len > 0 && fwrite(buf->b_u_line_ptr, (size_t)len, (size_t)1, fp) != 1)
+	return FAIL;
+    put_bytes(fp, (long_u)buf->b_u_line_lnum, 4);
+    put_bytes(fp, (long_u)buf->b_u_line_colnr, 4);
+
+    /* Undo structures header data */
+    put_header_ptr(fp, buf->b_u_oldhead);
+    put_header_ptr(fp, buf->b_u_newhead);
+    put_header_ptr(fp, buf->b_u_curhead);
+
+    put_bytes(fp, (long_u)buf->b_u_numhead, 4);
+    put_bytes(fp, (long_u)buf->b_u_seq_last, 4);
+    put_bytes(fp, (long_u)buf->b_u_seq_cur, 4);
+    put_time(fp, buf->b_u_seq_time);
+
+    return OK;
+}
+
+    static int
+serialize_uhp(fp, uhp)
+    FILE	*fp;
+    u_header_T	*uhp;
+{
+    int		i;
+    u_entry_T	*uep;
+
+    if (put_bytes(fp, (long_u)UF_HEADER_MAGIC, 2) == FAIL)
+	return FAIL;
+
+    put_header_ptr(fp, uhp->uh_next);
+    put_header_ptr(fp, uhp->uh_prev);
+    put_header_ptr(fp, uhp->uh_alt_next);
+    put_header_ptr(fp, uhp->uh_alt_prev);
+    put_bytes(fp, uhp->uh_seq, 4);
+    serialize_pos(uhp->uh_cursor, fp);
+#ifdef FEAT_VIRTUALEDIT
+    put_bytes(fp, (long_u)uhp->uh_cursor_vcol, 4);
+#else
+    put_bytes(fp, (long_u)0, 4);
+#endif
+    put_bytes(fp, (long_u)uhp->uh_flags, 2);
+    /* Assume NMARKS will stay the same. */
+    for (i = 0; i < NMARKS; ++i)
+	serialize_pos(uhp->uh_namedm[i], fp);
+#ifdef FEAT_VISUAL
+    serialize_visualinfo(&uhp->uh_visual, fp);
+#else
+    {
+	visualinfo_T info;
+
+	memset(&info, 0, sizeof(visualinfo_T));
+	serialize_visualinfo(&info, fp);
+    }
+#endif
+    put_time(fp, uhp->uh_time);
+
+    /* Write all the entries. */
+    for (uep = uhp->uh_entry; uep != NULL; uep = uep->ue_next)
+    {
+	put_bytes(fp, (long_u)UF_ENTRY_MAGIC, 2);
+	if (serialize_uep(uep, fp) == FAIL)
+	    return FAIL;
+    }
+    put_bytes(fp, (long_u)UF_ENTRY_END_MAGIC, 2);
+    return OK;
+}
+
+    static u_header_T *
+unserialize_uhp(fp, file_name)
+    FILE	*fp;
+    char_u	*file_name;
+{
+    u_header_T	*uhp;
+    int		i;
+    u_entry_T	*uep, *last_uep;
+    int		c;
+    int		error;
+
+    uhp = (u_header_T *)U_ALLOC_LINE(sizeof(u_header_T));
+    if (uhp == NULL)
+	return NULL;
+    vim_memset(uhp, 0, sizeof(u_header_T));
+#ifdef U_DEBUG
+    uhp->uh_magic = UH_MAGIC;
+#endif
+    /* We're not actually trying to store pointers here. We're just storing
+     * uh_seq numbers of the header pointed to, so we can swizzle them into
+     * pointers later - hence the type cast. */
+    uhp->uh_next = (u_header_T *)(long_u)get4c(fp);
+    uhp->uh_prev = (u_header_T *)(long_u)get4c(fp);
+    uhp->uh_alt_next = (u_header_T *)(long_u)get4c(fp);
+    uhp->uh_alt_prev = (u_header_T *)(long_u)get4c(fp);
+    uhp->uh_seq = get4c(fp);
+    if (uhp->uh_seq <= 0)
+    {
+	corruption_error("uh_seq", file_name);
+	vim_free(uhp);
+	return NULL;
+    }
+    unserialize_pos(&uhp->uh_cursor, fp);
+#ifdef FEAT_VIRTUALEDIT
+    uhp->uh_cursor_vcol = get4c(fp);
+#else
+    (void)get4c(fp);
+#endif
+    uhp->uh_flags = get2c(fp);
+    for (i = 0; i < NMARKS; ++i)
+	unserialize_pos(&uhp->uh_namedm[i], fp);
+#ifdef FEAT_VISUAL
+    unserialize_visualinfo(&uhp->uh_visual, fp);
+#else
+    {
+	visualinfo_T info;
+	unserialize_visualinfo(&info, fp);
+    }
+#endif
+    uhp->uh_time = get8ctime(fp);
+
+    /* Unserialize the uep list. */
+    last_uep = NULL;
+    while ((c = get2c(fp)) == UF_ENTRY_MAGIC)
+    {
+	error = FALSE;
+	uep = unserialize_uep(fp, &error, file_name);
+	if (last_uep == NULL)
+	    uhp->uh_entry = uep;
+	else
+	    last_uep->ue_next = uep;
+	last_uep = uep;
+	if (uep == NULL || error)
+	{
+	    u_free_uhp(uhp);
+	    return NULL;
+	}
+    }
+    if (c != UF_ENTRY_END_MAGIC)
+    {
+	corruption_error("entry end", file_name);
+	u_free_uhp(uhp);
+	return NULL;
+    }
+
+    return uhp;
+}
+
 /*
  * Serialize "uep" to "fp".
  */
@@ -830,7 +1000,6 @@ unserialize_uep(fp, error, file_name)
     char_u	*file_name;
 {
     int		i;
-    int		j;
     u_entry_T	*uep;
     char_u	**array;
     char_u	*line;
@@ -865,7 +1034,7 @@ unserialize_uep(fp, error, file_name)
     {
 	line_len = get4c(fp);
 	if (line_len >= 0)
-	    line = (char_u *)U_ALLOC_LINE(line_len + 1);
+	    line = read_string(fp, line_len);
 	else
 	{
 	    line = NULL;
@@ -876,9 +1045,6 @@ unserialize_uep(fp, error, file_name)
 	    *error = TRUE;
 	    return uep;
 	}
-	for (j = 0; j < line_len; j++)
-	    line[j] = getc(fp);
-	line[j] = NUL;
 	array[i] = line;
     }
     return uep;
@@ -910,9 +1076,15 @@ unserialize_pos(pos, fp)
     FILE  *fp;
 {
     pos->lnum = get4c(fp);
+    if (pos->lnum < 0)
+	pos->lnum = 0;
     pos->col = get4c(fp);
+    if (pos->col < 0)
+	pos->col = 0;
 #ifdef FEAT_VIRTUALEDIT
     pos->coladd = get4c(fp);
+    if (pos->coladd < 0)
+	pos->coladd = 0;
 #else
     (void)get4c(fp);
 #endif
@@ -975,9 +1147,8 @@ u_write_undo(name, forceit, buf, hash)
     char_u	*hash;
 {
     u_header_T	*uhp;
-    u_entry_T	*uep;
     char_u	*file_name;
-    int		str_len, i, mark;
+    int		mark;
 #ifdef U_DEBUG
     int		headers_written = 0;
 #endif
@@ -1069,7 +1240,8 @@ u_write_undo(name, forceit, buf, hash)
 		    {
 			if (name == NULL)
 			    verbose_enter();
-			smsg((char_u *)_("Will not overwrite, this is not an undo file: %s"),
+			smsg((char_u *)
+			_("Will not overwrite, this is not an undo file: %s"),
 								   file_name);
 			if (name == NULL)
 			    verbose_leave();
@@ -1083,7 +1255,7 @@ u_write_undo(name, forceit, buf, hash)
 
     /* If there is no undo information at all, quit here after deleting any
      * existing undo file. */
-    if (buf->b_u_numhead == 0)
+    if (buf->b_u_numhead == 0 && buf->b_u_line_ptr == NULL)
     {
 	if (p_verbose > 0)
 	    verb_msg((char_u *)_("Skipping undo file write, noting to undo"));
@@ -1141,37 +1313,12 @@ u_write_undo(name, forceit, buf, hash)
     /* Undo must be synced. */
     u_sync(TRUE);
 
-    /* Start writing, first the undo file header. */
-    if (fwrite(UF_START_MAGIC, (size_t)UF_START_MAGIC_LEN, (size_t)1, fp) != 1)
-	goto write_error;
-    put_bytes(fp, (long_u)UF_VERSION, 2);
-
-    /* Write a hash of the buffer text, so that we can verify it is still the
-     * same when reading the buffer text. */
-    if (fwrite(hash, (size_t)UNDO_HASH_SIZE, (size_t)1, fp) != 1)
-	goto write_error;
-    put_bytes(fp, (long_u)buf->b_ml.ml_line_count, 4);
-
-    /* Begin undo data for U */
-    str_len = buf->b_u_line_ptr != NULL ? (int)STRLEN(buf->b_u_line_ptr) : 0;
-    put_bytes(fp, (long_u)str_len, 4);
-    if (str_len > 0 && fwrite(buf->b_u_line_ptr, (size_t)str_len,
-							  (size_t)1, fp) != 1)
+    /*
+     * Write the header.
+     */
+    if (serialize_header(fp, buf, hash) == FAIL)
 	goto write_error;
 
-    put_bytes(fp, (long_u)buf->b_u_line_lnum, 4);
-    put_bytes(fp, (long_u)buf->b_u_line_colnr, 4);
-
-    /* Begin general undo data */
-    put_header_ptr(fp, buf->b_u_oldhead);
-    put_header_ptr(fp, buf->b_u_newhead);
-    put_header_ptr(fp, buf->b_u_curhead);
-
-    put_bytes(fp, (long_u)buf->b_u_numhead, 4);
-    put_bytes(fp, (long_u)buf->b_u_seq_last, 4);
-    put_bytes(fp, (long_u)buf->b_u_seq_cur, 4);
-    put_time(fp, buf->b_u_seq_time);
-
     /*
      * Iteratively serialize UHPs and their UEPs from the top down.
      */
@@ -1186,48 +1333,11 @@ u_write_undo(name, forceit, buf, hash)
 #ifdef U_DEBUG
 	    ++headers_written;
 #endif
-
-	    if (put_bytes(fp, (long_u)UF_HEADER_MAGIC, 2) == FAIL)
+	    if (serialize_uhp(fp, uhp) == FAIL)
 		goto write_error;
-
-	    put_header_ptr(fp, uhp->uh_next);
-	    put_header_ptr(fp, uhp->uh_prev);
-	    put_header_ptr(fp, uhp->uh_alt_next);
-	    put_header_ptr(fp, uhp->uh_alt_prev);
-            put_bytes(fp, uhp->uh_seq, 4);
-            serialize_pos(uhp->uh_cursor, fp);
-#ifdef FEAT_VIRTUALEDIT
-            put_bytes(fp, (long_u)uhp->uh_cursor_vcol, 4);
-#else
-            put_bytes(fp, (long_u)0, 4);
-#endif
-            put_bytes(fp, (long_u)uhp->uh_flags, 2);
-            /* Assume NMARKS will stay the same. */
-            for (i = 0; i < NMARKS; ++i)
-                serialize_pos(uhp->uh_namedm[i], fp);
-#ifdef FEAT_VISUAL
-            serialize_visualinfo(&uhp->uh_visual, fp);
-#else
-	    {
-		visualinfo_T info;
-
-		memset(&info, 0, sizeof(visualinfo_T));
-		serialize_visualinfo(&info, fp);
-	    }
-#endif
-            put_time(fp, uhp->uh_time);
-
-	    /* Write all the entries. */
-	    for (uep = uhp->uh_entry; uep != NULL; uep = uep->ue_next)
-	    {
-		put_bytes(fp, (long_u)UF_ENTRY_MAGIC, 2);
-                if (serialize_uep(uep, fp) == FAIL)
-		    goto write_error;
-	    }
-	    put_bytes(fp, (long_u)UF_ENTRY_END_MAGIC, 2);
         }
 
-        /* Now walk through the tree - algorithm from undo_time */
+        /* Now walk through the tree - algorithm from undo_time(). */
         if (uhp->uh_prev != NULL && uhp->uh_prev->uh_walk != mark)
             uhp = uhp->uh_prev;
         else if (uhp->uh_alt_next != NULL && uhp->uh_alt_next->uh_walk != mark)
@@ -1255,6 +1365,8 @@ write_error:
         EMSG2(_("E829: write error in undo file: %s"), file_name);
 
 #if defined(MACOS_CLASSIC) || defined(WIN3264)
+    /* Copy file attributes; for systems where this can only be done after
+     * closing the file. */
     if (buf->b_ffname != NULL)
 	(void)mch_copy_file_attribute(buf->b_ffname, file_name);
 #endif
@@ -1282,9 +1394,10 @@ theend:
  * "hash[UNDO_HASH_SIZE]" must be the hash value of the buffer text.
  */
     void
-u_read_undo(name, hash)
+u_read_undo(name, hash, orig_name)
     char_u *name;
     char_u *hash;
+    char_u *orig_name;
 {
     char_u	*file_name;
     FILE	*fp;
@@ -1301,7 +1414,6 @@ u_read_undo(name, hash)
     time_t	seq_time;
     int		i, j;
     int		c;
-    u_entry_T	*uep, *last_uep;
     u_header_T	*uhp;
     u_header_T	**uhp_table = NULL;
     char_u	read_hash[UNDO_HASH_SIZE];
@@ -1309,12 +1421,34 @@ u_read_undo(name, hash)
 #ifdef U_DEBUG
     int		*uhp_table_used;
 #endif
+#ifdef UNIX
+    struct stat	st_orig;
+    struct stat	st_undo;
+#endif
 
     if (name == NULL)
     {
         file_name = u_get_undo_file_name(curbuf->b_ffname, TRUE);
 	if (file_name == NULL)
 	    return;
+
+#ifdef UNIX
+	/* For safety we only read an undo file if the owner is equal to the
+	 * owner of the text file. */
+	if (mch_stat((char *)orig_name, &st_orig) >= 0
+		&& mch_stat((char *)file_name, &st_undo) >= 0
+		&& st_orig.st_uid != st_undo.st_uid)
+	{
+	    if (p_verbose > 0)
+	    {
+		verbose_enter();
+		smsg((char_u *)_("Not reading undo file, owner differs: %s"),
+								   file_name);
+		verbose_leave();
+	    }
+	    return;
+	}
+#endif
     }
     else
         file_name = name;
@@ -1325,6 +1459,7 @@ u_read_undo(name, hash)
 	smsg((char_u *)_("Reading undo file: %s"), file_name);
 	verbose_leave();
     }
+
     fp = mch_fopen((char *)file_name, "r");
     if (fp == NULL)
     {
@@ -1362,27 +1497,27 @@ u_read_undo(name, hash)
         {
 	    if (name == NULL)
 		verbose_enter();
-            give_warning((char_u *)_("File contents changed, cannot use undo info"), TRUE);
+            give_warning((char_u *)
+		      _("File contents changed, cannot use undo info"), TRUE);
 	    if (name == NULL)
 		verbose_leave();
         }
         goto error;
     }
 
-    /* Begin undo data for U */
+    /* Read undo data for "U" command. */
     str_len = get4c(fp);
     if (str_len < 0)
         goto error;
-    else if (str_len > 0)
-    {
-        if ((line_ptr = U_ALLOC_LINE(str_len + 1)) == NULL)
-            goto error;
-        for (i = 0; i < str_len; i++)
-            line_ptr[i] = (char_u)getc(fp);
-        line_ptr[i] = NUL;
-    }
+    if (str_len > 0)
+	line_ptr = read_string(fp, str_len);
     line_lnum = (linenr_T)get4c(fp);
     line_colnr = (colnr_T)get4c(fp);
+    if (line_lnum < 0 || line_colnr < 0)
+    {
+	corruption_error("line lnum/col", file_name);
+	goto error;
+    }
 
     /* Begin general undo data */
     old_header_seq = get4c(fp);
@@ -1409,76 +1544,13 @@ u_read_undo(name, hash)
     {
 	if (num_read_uhps >= num_head)
 	{
-	    corruption_error("num_head", file_name);
+	    corruption_error("num_head too small", file_name);
 	    goto error;
 	}
 
-        uhp = (u_header_T *)U_ALLOC_LINE(sizeof(u_header_T));
-        if (uhp == NULL)
-            goto error;
-        vim_memset(uhp, 0, sizeof(u_header_T));
-#ifdef U_DEBUG
-	uhp->uh_magic = UH_MAGIC;
-#endif
-        /* We're not actually trying to store pointers here. We're just storing
-         * IDs so we can swizzle them into pointers later - hence the type
-	 * cast. */
-        uhp->uh_next = (u_header_T *)(long_u)get4c(fp);
-        uhp->uh_prev = (u_header_T *)(long_u)get4c(fp);
-        uhp->uh_alt_next = (u_header_T *)(long_u)get4c(fp);
-        uhp->uh_alt_prev = (u_header_T *)(long_u)get4c(fp);
-        uhp->uh_seq = get4c(fp);
-        if (uhp->uh_seq <= 0)
-        {
-	    corruption_error("uh_seq", file_name);
-            vim_free(uhp);
-            goto error;
-        }
-        uhp->uh_walk = 0;
-        unserialize_pos(&uhp->uh_cursor, fp);
-#ifdef FEAT_VIRTUALEDIT
-        uhp->uh_cursor_vcol = get4c(fp);
-#else
-        (void)get4c(fp);
-#endif
-        uhp->uh_flags = get2c(fp);
-        for (i = 0; i < NMARKS; ++i)
-            unserialize_pos(&uhp->uh_namedm[i], fp);
-#ifdef FEAT_VISUAL
-        unserialize_visualinfo(&uhp->uh_visual, fp);
-#else
-	{
-	    visualinfo_T info;
-	    unserialize_visualinfo(&info, fp);
-	}
-#endif
-        uhp->uh_time = get8ctime(fp);
-
-        /* Unserialize the uep list. */
-        last_uep = NULL;
-        while ((c = get2c(fp)) == UF_ENTRY_MAGIC)
-        {
-	    int error = FALSE;
-
-	    uep = unserialize_uep(fp, &error, file_name);
-            if (last_uep == NULL)
-                uhp->uh_entry = uep;
-	    else
-                last_uep->ue_next = uep;
-            last_uep = uep;
-	    if (uep == NULL || error)
-	    {
-		u_free_uhp(uhp);
-                goto error;
-	    }
-        }
-	if (c != UF_ENTRY_END_MAGIC)
-        {
-	    corruption_error("entry end", file_name);
-            u_free_uhp(uhp);
-            goto error;
-        }
-
+	uhp = unserialize_uhp(fp, file_name);
+	if (uhp == NULL)
+	    goto error;
 	uhp_table[num_read_uhps++] = uhp;
     }
 
@@ -1487,7 +1559,6 @@ u_read_undo(name, hash)
 	corruption_error("num_head", file_name);
 	goto error;
     }
-
     if (c != UF_HEADER_END_MAGIC)
     {
 	corruption_error("end marker", file_name);
@@ -1502,8 +1573,8 @@ u_read_undo(name, hash)
 # define SET_FLAG(j)
 #endif
 
-    /* We have put all of the uhps into a table. Now we iterate through the
-     * table and swizzle each sequence number we've stored in uh_* into a
+    /* We have put all of the headers into a table. Now we iterate through the
+     * table and swizzle each sequence number we have stored in uh_* into a
      * pointer corresponding to the header with that sequence number. */
     for (i = 0; i < num_head; i++)
     {
@@ -1519,7 +1590,6 @@ u_read_undo(name, hash)
 		corruption_error("duplicate uh_seq", file_name);
                 goto error;
 	    }
-
             if (uhp_table[j]->uh_seq == (long)uhp->uh_next)
 	    {
                 uhp->uh_next = uhp_table[j];