[27248] in Source-Commits

home help back first fref pref prev next nref lref last post

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);
>

home help back first fref pref prev next nref lref last post