Re: [PATCH] setup.exe SEGV on WinXP/Pro
On Aug 11 19:24, Achim Gratz wrote: Corinna Vinschen writes: Patch applied. As a follow-up on the discussion I've checked why this is a conversion operator in the first place and the answer is no particularly good reason. I propose to keep the implementation unchanged, but as a plain member function str() instead, which should be easier to grok some time later. This is the only path the conversion operator got invoked through anyway. Applied. Thanks, Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat pgpJacBM2DzBD.pgp Description: PGP signature
Re: [PATCH] setup.exe SEGV on WinXP/Pro
Corinna Vinschen writes:
Patch applied.
As a follow-up on the discussion I've checked why this is a conversion
operator in the first place and the answer is no particularly good
reason. I propose to keep the implementation unchanged, but as a plain
member function str() instead, which should be easier to grok some time
later. This is the only path the conversion operator got invoked
through anyway.
From 296273b76f6913c49bab363b08fb18865eec4351 Mon Sep 17 00:00:00 2001
From: Achim Gratz strom...@stromeko.de
Date: Sat, 10 Aug 2013 15:24:41 +0200
Subject: [PATCH] remove conversion operator std::string(), instead use its
implementation for str() member function
* csu_util/MD5Sum.h (MD5Sum): Remove declaration for conversion
operator std::string(). Remove implementation of member
function str() using the conversion operator.
* csu_util/MD5Sum.cc (MD5Sum::str): Reuse implementation of
conversion operator std::String to implement member function
str() with.
---
csu_util/MD5Sum.cc | 3 ++-
csu_util/MD5Sum.h | 3 +--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/csu_util/MD5Sum.cc b/csu_util/MD5Sum.cc
index eb679b5..1d9c362 100644
--- a/csu_util/MD5Sum.cc
+++ b/csu_util/MD5Sum.cc
@@ -80,7 +80,8 @@ MD5Sum::finish()
delete internalData; internalData = 0;
}
-MD5Sum::operator std::string() const
+std::string
+MD5Sum::str() const
{
std::ostringstream hexdigest;
diff --git a/csu_util/MD5Sum.h b/csu_util/MD5Sum.h
index ff9021e..5c51bbc 100644
--- a/csu_util/MD5Sum.h
+++ b/csu_util/MD5Sum.h
@@ -44,8 +44,7 @@ class MD5Sum
void finish();
bool isSet() const { return (state == Set); };
-operator std::string() const;
-std::string str() const { return (std::string)(*this); };
+std::string str() const;
bool operator == (const MD5Sum other) const;
bool operator != (const MD5Sum other) const { return !(*this == other); };
--
1.8.3.1
Regards,
Achim.
--
+[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]+
SD adaptations for Waldorf Q V3.00R3 and Q+ V3.54R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada
Re: [PATCH] setup.exe SEGV on WinXP/Pro
On Aug 8 20:34, Achim Gratz wrote: I've been having sporadic SEGV on WinXP/Pro just after the MD5 of a package was checked that used to clear up after a reboot. Today, with a freshly built setup.exe this failure was now entirely reproduceable. I've fixed it by reimplementing the string formatting for the MD5 digest using C++ stream functions. From 677e2e89d1e4046c967dd1759ac53116f6643bd9 Mon Sep 17 00:00:00 2001 From: Achim Gratz strom...@stromeko.de Date: Thu, 8 Aug 2013 20:23:31 +0200 Subject: [PATCH] fix SEGV on WinXP/Pro * csu_util/MD5Sum.cc (MD5Sum::operator std::string() const): Reimplement using stringstream to avoid a SEGV on WinXP/Pro. Patch applied. - return std::string(hexdigest); ^^ I'm wondering if that was the problem. This expression constructs a std:string and then immediately destructs it since the scope is limited to the end of the function (which the return statement is all about). Reading the value of this object in the parent function is basically luck, isn't it? Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat
Re: [PATCH] setup.exe SEGV on WinXP/Pro
On Fri, Aug 09, 2013 at 11:07:26AM +0200, Corinna Vinschen wrote: On Aug 8 20:34, Achim Gratz wrote: I've been having sporadic SEGV on WinXP/Pro just after the MD5 of a package was checked that used to clear up after a reboot. Today, with a freshly built setup.exe this failure was now entirely reproduceable. I've fixed it by reimplementing the string formatting for the MD5 digest using C++ stream functions. From 677e2e89d1e4046c967dd1759ac53116f6643bd9 Mon Sep 17 00:00:00 2001 From: Achim Gratz strom...@stromeko.de Date: Thu, 8 Aug 2013 20:23:31 +0200 Subject: [PATCH] fix SEGV on WinXP/Pro * csu_util/MD5Sum.cc (MD5Sum::operator std::string() const): Reimplement using stringstream to avoid a SEGV on WinXP/Pro. Patch applied. - return std::string(hexdigest); ^^ I'm wondering if that was the problem. This expression constructs a std:string and then immediately destructs it since the scope is limited to the end of the function (which the return statement is all about). Reading the value of this object in the parent function is basically luck, isn't it? Sheesh. Yes, that looks like the problem. But doesn't the new code do pretty much the same thing? + std::ostringstream hexdigest; + return hexdigest.str(); cgf
RE: [PATCH] setup.exe SEGV on WinXP/Pro
Christopher Faylor wrote on 2013-08-09: On Fri, Aug 09, 2013 at 11:07:26AM +0200, Corinna Vinschen wrote: On Aug 8 20:34, Achim Gratz wrote: I've been having sporadic SEGV on WinXP/Pro just after the MD5 of a package was checked that used to clear up after a reboot. Today, with a freshly built setup.exe this failure was now entirely reproduceable. I've fixed it by reimplementing the string formatting for the MD5 digest using C++ stream functions. From 677e2e89d1e4046c967dd1759ac53116f6643bd9 Mon Sep 17 00:00:00 2001 From: Achim Gratz strom...@stromeko.de Date: Thu, 8 Aug 2013 20:23:31 +0200 Subject: [PATCH] fix SEGV on WinXP/Pro * csu_util/MD5Sum.cc (MD5Sum::operator std::string() const): Reimplement using stringstream to avoid a SEGV on WinXP/Pro. Patch applied. - return std::string(hexdigest); ^^ I'm wondering if that was the problem. This expression constructs a std:string and then immediately destructs it since the scope is limited to the end of the function (which the return statement is all about). Reading the value of this object in the parent function is basically luck, isn't it? Sheesh. Yes, that looks like the problem. But doesn't the new code do pretty much the same thing? + std::ostringstream hexdigest; + return hexdigest.str(); According to this: http://stackoverflow.com/questions/275214/scope-and-return-values-in-c Returning the object should be ok because it is copied before leaving the function scope; returning a reference or pointer to the object is where you get into problems. -- Bryan Thrall Principal Software Engineer FlightSafety International * Visual Simulation Systems * 5695 Campus Parkway * Hazelwood, MO 63042 Tel: (314) 551-8413 * Fax: (314) 551-8444 bryan.thr...@flightsafety.com * www.flightsafety.com * A Berkshire Hathaway company
Re: [PATCH] setup.exe SEGV on WinXP/Pro
On Aug 9 12:55, Christopher Faylor wrote: On Fri, Aug 09, 2013 at 11:01:32AM -0500, Thrall, Bryan wrote: Christopher Faylor wrote on 2013-08-09: On Fri, Aug 09, 2013 at 11:07:26AM +0200, Corinna Vinschen wrote: On Aug 8 20:34, Achim Gratz wrote: I've been having sporadic SEGV on WinXP/Pro just after the MD5 of a package was checked that used to clear up after a reboot. Today, with a freshly built setup.exe this failure was now entirely reproduceable. I've fixed it by reimplementing the string formatting for the MD5 digest using C++ stream functions. From 677e2e89d1e4046c967dd1759ac53116f6643bd9 Mon Sep 17 00:00:00 2001 From: Achim Gratz strom...@stromeko.de Date: Thu, 8 Aug 2013 20:23:31 +0200 Subject: [PATCH] fix SEGV on WinXP/Pro * csu_util/MD5Sum.cc (MD5Sum::operator std::string() const): Reimplement using stringstream to avoid a SEGV on WinXP/Pro. Patch applied. - return std::string(hexdigest); ^^ I'm wondering if that was the problem. This expression constructs a std:string and then immediately destructs it since the scope is limited to the end of the function (which the return statement is all about). Reading the value of this object in the parent function is basically luck, isn't it? Sheesh. Yes, that looks like the problem. But doesn't the new code do pretty much the same thing? + std::ostringstream hexdigest; + return hexdigest.str(); According to this: http://stackoverflow.com/questions/275214/scope-and-return-values-in-c Returning the object should be ok because it is copied before leaving the function scope; returning a reference or pointer to the object is where you get into problems. Thanks for clarifying. Isn't that what the original code did too then? Not quite. ostringstream::str returns string, the string constructor implicitely returns string. It's sometimes tricky to wrap the brain around the differences as far as the scope is concerned. Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat pgpCuWNqsnEoF.pgp Description: PGP signature
Re: [PATCH] setup.exe SEGV on WinXP/Pro
Corinna Vinschen writes: - return std::string(hexdigest); ^^ I'm wondering if that was the problem. I was wondering about this, too — but then the SEGV should have happened in the calling function. I believe that the vetting of hexdigest[32] with a zero was the actual point of failure. That really should not happen, I think, but then I haven't chased it down to the actual instructions. This expression constructs a std:string and then immediately destructs it since the scope is limited to the end of the function (which the return statement is all about). This being C++, the compiler is expected to copy the temporary object to the return object or (as would be possible here) to construct the temporary object in the place of the return object, thereby saving the copy operation. Reading the value of this object in the parent function is basically luck, isn't it? If it was just giving out a char* that would be a common mistake to make, but not much of C++ would work if that was the case when an actual object is involved. Regards, Achim. -- +[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]+ Wavetables for the Waldorf Blofeld: http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables
Re: [PATCH] setup.exe SEGV on WinXP/Pro
On Aug 9 13:54, Christopher Faylor wrote: On Fri, Aug 09, 2013 at 07:47:23PM +0200, Corinna Vinschen wrote: On Aug 9 12:55, Christopher Faylor wrote: On Fri, Aug 09, 2013 at 11:01:32AM -0500, Thrall, Bryan wrote: Christopher Faylor wrote on 2013-08-09: On Fri, Aug 09, 2013 at 11:07:26AM +0200, Corinna Vinschen wrote: On Aug 8 20:34, Achim Gratz wrote: I've been having sporadic SEGV on WinXP/Pro just after the MD5 of a package was checked that used to clear up after a reboot. Today, with a freshly built setup.exe this failure was now entirely reproduceable. I've fixed it by reimplementing the string formatting for the MD5 digest using C++ stream functions. From 677e2e89d1e4046c967dd1759ac53116f6643bd9 Mon Sep 17 00:00:00 2001 From: Achim Gratz strom...@stromeko.de Date: Thu, 8 Aug 2013 20:23:31 +0200 Subject: [PATCH] fix SEGV on WinXP/Pro * csu_util/MD5Sum.cc (MD5Sum::operator std::string() const): Reimplement using stringstream to avoid a SEGV on WinXP/Pro. Patch applied. - return std::string(hexdigest); ^^ I'm wondering if that was the problem. This expression constructs a std:string and then immediately destructs it since the scope is limited to the end of the function (which the return statement is all about). Reading the value of this object in the parent function is basically luck, isn't it? Sheesh. Yes, that looks like the problem. But doesn't the new code do pretty much the same thing? + std::ostringstream hexdigest; + return hexdigest.str(); According to this: http://stackoverflow.com/questions/275214/scope-and-return-values-in-c Returning the object should be ok because it is copied before leaving the function scope; returning a reference or pointer to the object is where you get into problems. Thanks for clarifying. Isn't that what the original code did too then? Not quite. ostringstream::str returns string, the string constructor implicitely returns string. It's sometimes tricky to wrap the brain around the differences as far as the scope is concerned. Perhaps a comment here would help future perusers of this function. In theory it's basic C++ behaviour, but comments don't hurt, so, sure. Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat pgpUGI_O02W8g.pgp Description: PGP signature
Re: [PATCH] setup.exe SEGV on WinXP/Pro
Corinna Vinschen writes: Not quite. ostringstream::str returns string, the string constructor implicitely returns string. I could be reading it wrong, but I don't think that's what the C++11 standard says should happen. The implicit copy constructor transfering the return value out of the function will treat the argument of the return as an rvalue reference (but it'd do that just the same for ostringstream::str, since that's simply how it is declared. It's sometimes tricky to wrap the brain around the differences as far as the scope is concerned. It is true that the temporary object created by the explicit constructor in the return statement is destroyed at the end of full-expression of the return (12.2, 12.4). But, the return statement itself has initialization semantics, which means it operates as if a copy constructor was invoked between the argument of the return and the actual return value of the function (which in this case is a conversion function, but I didn't find any hints in the standard that this would somehow change semantics). Regards, Achim. -- +[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]+ Samples for the Waldorf Blofeld: http://Synth.Stromeko.net/Downloads.html#BlofeldSamplesExtra
