emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Patches with independent changes


From: Eli Zaretskii
Subject: Re: Patches with independent changes
Date: Sat, 25 Jan 2014 09:22:02 +0200

> Date: Fri, 24 Jan 2014 14:34:35 -0800
> From: Paul Eggert <address@hidden>
> CC: address@hidden
> 
> On 01/24/2014 01:39 PM, Eli Zaretskii wrote:
> 
> > they both address the fact that fchmod is not available on Windows. 
> > One without the other would break the build on Windows.
> 
> I wasn't referring to two hunks in the patch. I was referring to two of 
> the changes installed by that patch. One change worked around 
> WINDOWSNT's lack of fchmod, and the other replaced "!=" with "<".The 
> changes were independent and only the first change was needed to fix the 
> bug.

There was no second change.  There were 2 hunks in a single change
that fixed a single problem:

=== modified file 'lib-src/update-game-score.c'
--- lib-src/update-game-score.c 2014-01-22 19:02:41 +0000
+++ lib-src/update-game-score.c 2014-01-22 19:38:31 +0000
@@ -443,8 +443,10 @@ write_scores (const char *filename, cons
   fd = mkostemp (tempfile, 0);
   if (fd < 0)
     return -1;
+#ifndef WINDOWSNT
   if (fchmod (fd, 0644) != 0)
     return -1;
+#endif
   f = fdopen (fd, "w");
   if (! f)
     return -1;
@@ -457,6 +459,10 @@ write_scores (const char *filename, cons
     return -1;
   if (rename (tempfile, filename) != 0)
     return -1;
+#ifdef WINDOWSNT
+  if (chmod (filename, 0644) < 0)
+    return -1;
+#endif
   return 0;
 }
 
I didn't change anything in the call to fchmod, except conditioning it
on non-Windows build.  The code that calls chmod instead was added,
and it used < from the get-go.  So there's no second change, the two
hunks were strictly required to make the code compile on Windows.

> > That I used < rather than != is immaterial
> 
> You thought replacing "!=" with "<" was needed, so the intent of that 
> patch was to install multiple independent fixes. Which was fine.

I didn't replace anything, the line with chmod was _added_ in its
entirety.

> Here's another example. Trunk bzr 116064 fixes a file name handling 
> failure on MS-Windows 9X. While it's in the neighborhood, it makes the 
> independent changes of removing initialization code for the variable 
> g_b_init_is_w9x, and of removing that variable's definition. These 
> independent changes weren't needed to fix the bug.

Yes, they were needed: that variable was no longer used after the
change.  Removal of unused code is the same as changing commentary or
whitespace: it is generally done when the related code changes.

> This sort of thing is quite common, and there's nothing wrong with it.

Indeed, there's nothing wrong with the changes you are trying to
represent as examples.  Your changes were different in kind, and that
is what this discussion is about.

> >> [retitling from "Changes in update-game-score.c"]
> > [Why?]
> 
> Others requested retitling and this seemed reasonable.

Others were wrong, there's no reason to retitle.  The discussion is
still about changes in update-game-score.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]