[287] in athena10
Re: /svn/athena r23051 - in trunk/athena/bin/discuss: client
daemon@ATHENA.MIT.EDU (Greg Hudson)
Mon Jun 30 13:21:11 2008
From: Greg Hudson <ghudson@MIT.EDU>
To: Greg Price <price@mit.edu>
Cc: athena10@mit.edu
In-Reply-To: <20080630171054.GQ5106@vinegar-pot.mit.edu>
Content-Type: text/plain
Date: Mon, 30 Jun 2008 13:20:26 -0400
Message-Id: <1214846426.18347.167.camel@error-messages.mit.edu>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
On Mon, 2008-06-30 at 13:10 -0400, Greg Price wrote:
> > - if (*(char **)info)
> > - free (*(char **)info);
> > + if (*info)
> > + free (*infop);
> ^^^^ ^
>
> Is this right?
>
> I see there's an 'infop' further up, and I haven't read the actual
> file, so for all I know it is. Seems odd, though.
It's right, though I agree it looks wrong.
Discuss has a very simple file super-abstraction called "tfile". A
tfile has two implementation data fields: a pointer bit (infop) and an
integer bit (info).
This particular module stores "files" in iovec memory buffers (lists of
strings). You can either pass in a single string (mem_tfile) in which
case memory for the iovec is allocated by tmem, or you can pass in an
iovec (memv_tfile), in which case the caller manages the iovec memory.
The iovec is stored in the infop field. At destruction time, we need to
know whether the iovec should be freed or not.
The old way of solving this problem was to stuff a copy of the infop
field into the info field for mem_tfile, and to set the info field to 0
for memv_tfile. That doesn't work on 64-bit platforms since pointers
don't fit into ints. The new method is to treat the info field as a
binary flag indicating whether the infop field should be freed at
destruction time or not.
In a real code base this should all be clarified in comments, but the
discuss code base is largely uncommented, and fixing that in one
isolated area doesn't seem worth it.
