[27248] in Source-Commits
Re: discuss commit: Fix segmentation fault on listing transactions
daemon@ATHENA.MIT.EDU (Alex Chernyakhovsky)
Sun Aug 25 13:59:40 2013
MIME-Version: 1.0
In-Reply-To: <3A12D5F0-67BB-4AAA-8E2A-AFFF969711C4@mit.edu>
Date: Sun, 25 Aug 2013 13:59:31 -0400
Message-ID: <CAB18ysq660MWsGo8kv2LDPM_p3FbbrksBi+_a2sP-aYPjHoJTg@mail.gmail.com>
From: Alex Chernyakhovsky <achernya@MIT.EDU>
To: Jonathan Reed <jdreed@MIT.EDU>
Cc: Victor Vasiliev <vasilvv@MIT.EDU>,
"source-commits@mit.edu" <source-commits@MIT.EDU>
Content-Type: text/plain; charset=ISO-8859-1
I would support moving to the +/- system (specifically, the one
Scripts uses as per /mit/scripts/doc/scripts-code-review) if we don't
think it's overkill. It looks like the previous ACK/NAK system maps
onto "+1", and "-1, revert", so this gives us more flexibility if we
think it's useful. I suspect it probably is useful, but I've also not
heard any complaints about the previous system.
Sincerely,
-Alex
On Sun, Aug 25, 2013 at 10:57 AM, Jonathan Reed <jdreed@mit.edu> wrote:
> I don't love the fix, but I also don't love anything else in the Discuss code. ACK, I think.
>
> If we were moving to the +/- system (are we?), I'd probably give it a +0, though "not segfaulting" is still an improvement.
>
> -Jon
>
> On Aug 25, 2013, at 4:00 AM, Victor Vasiliev wrote:
>
>> https://github.com/mit-athena/discuss/commit/b62bbe881f0e2058f0b608a25840add2c8af71a0
>> commit b62bbe881f0e2058f0b608a25840add2c8af71a0
>> Author: Victor Vasiliev <vasilvv@mit.edu>
>> Date: Sun Aug 25 03:59:58 2013 -0400
>>
>> Fix segmentation fault on listing transactions
>>
>> Due to the fact that in readstr() the pointer to memory allocated
>> on stack could be replaced with a pointer to string constant, discuss(1)
>> failed when listing of transactions was interrupted by Ctrl+C. This
>> commit fixes that, probably not in the best possible way.
>>
>> debian/changelog | 7 ++++++-
>> libds/rpcall.c | 5 ++++-
>> 2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/debian/changelog b/debian/changelog
>> index 2230b75..130d2b2 100644
>> --- a/debian/changelog
>> +++ b/debian/changelog
>> @@ -1,9 +1,14 @@
>> -debathena-discuss (10.0.15-0debathena3) UNRELEASED; urgency=low
>> +debathena-discuss (10.0.16-0debathena1) UNRELEASED; urgency=low
>>
>> + [ Jonathan Reed ]
>> * Switch from control.in to control (Trac: #561)
>> * Bump Standards-Version to 3.9.3
>> * Bump compat level to 7
>>
>> + [ Victor Vasiliev ]
>> + * Fix segmentation fault caused by interrupting listing of the transactions
>> + in the meeting.
>> +
>> -- Jonathan Reed <jdreed@mit.edu> Wed, 03 Apr 2013 13:59:39 -0400
>>
>> debathena-discuss (10.0.15-0debathena2) unstable; urgency=low
>> diff --git a/libds/rpcall.c b/libds/rpcall.c
>> index d5b4a5a..1d0cae7 100644
>> --- a/libds/rpcall.c
>> +++ b/libds/rpcall.c
>> @@ -501,7 +501,10 @@ char *recvstr ()
>>
>> if (USP_get_string(us, &str) != SUCCESS) {
>> rpc_err = errno;
>> - return("");
>> + /* the invoking code will call free()
>> + * on values returned from this function */
>> + str = malloc(1);
>> + str[0] = '\0';
>> }
>>
>> return (str);
>