socketRecordSize is probably not 64-bit friendly

On Sun, Mar 22, 2015 at 11:27:05AM -0700, Eliot Miranda wrote:
>
> Hi David,
>
> On Mar 22, 2015, at 10:58 AM, "David T. Lewis" <[hidden email]> wrote:
>
> >
> > On Sun, Mar 22, 2015 at 05:04:12PM +0100, Nicolas Cellier wrote:
> >>
> >> Hi,
> >> I noticed that SocketPlugin>>socketRecordSize was so defined:
> >> socketRecordSize
> >>    "Return the size of a Smalltalk socket record in bytes."
> >>
> >>    ^ self cCode: 'sizeof(SQSocket)' inSmalltalk: [12]
> >>
> >>
> >> Shouldn't it have been like its brothers in OSProcessPlugin and AioPlugin:
> >>
> >> socketRecordSize
> >>    "Return the size of a Smalltalk socket record in bytes."
> >>
> >>    ^ self cCode: 'sizeof(SQSocket)' inSmalltalk: [Smalltalk wordSize * 3]
> >>
> >>
> >> I presume we don't use self sizeof: because the Smalltalk side does not
> >> have any idea of what a SQSocket is... (an opaque struct/handle)
> >
> > On my machine, sizeof(SQSocket) is 16 for both 32 bits and 64 bits. I would
> > have expected it to be 12 for the 32-bit case, but maybe the compiler is
> > aligning it differently for some reason.
> >
> > In any case, "Smalltalk wordSize * 3" would not be right, because the
> > actual data structure is this:
> >
> >    typedef struct
> >    {
> >      int   sessionID;
> >      int   socketType;  /* 0 = TCP, 1 = UDP */
> >      void  *privateSocketPtr;
> >    }  SQSocket, *SocketPtr;
> >
> >
> > In order to cover the common cases, it might be best to just change
> > the implementation to this:
> >
> >     ^ self cCode: 'sizeof(SQSocket)' inSmalltalk: [16]
>
> No, no and thrice no!  :). It should /always/ be written
>
>      ^self sizeof: #SQSocket
>
> and the size answered in a sizeof: implementation marked <doNotGenerate>.  That way
> - there is one def not one in potentially multiple clients
> - you don't even need the sizeOfSQSocket method in the first place.
>
> Please see VMMaker.oscog
>

Ok.

I note for the record that SocketPlugin>>socketRecordSize has a time stamp
of 2/22/2000 ;-)

On Mon, Mar 23, 2015 at 10:22:00AM -0700, Eliot Miranda wrote:
>
> On Sun, Mar 22, 2015 at 11:43 AM, David T. Lewis <[hidden email]>
> wrote:
> >
> > On Sun, Mar 22, 2015 at 11:27:05AM -0700, Eliot Miranda wrote:
> > >
> > > No, no and thrice no!  :). It should /always/ be written
> > >
> > >      ^self sizeof: #SQSocket
> > >
> > > and the size answered in a sizeof: implementation marked
> > <doNotGenerate>.  That way
> > > - there is one def not one in potentially multiple clients
> > > - you don't even need the sizeOfSQSocket method in the first place.
> > >
> > > Please see VMMaker.oscog
> > >
> >
> > Ok.
> >
> > I note for the record that SocketPlugin>>socketRecordSize has a time stamp
> > of 2/22/2000 ;-)
> >
>
> :)
>
> Please find a version of the AioPlugin that has this change attached.  I
> can't commit to the repo.  I hope you'll find it fit to upload yourself :)


Thanks Eliot,

VMConstruction-Plugins-AioPlugin-eem.18 is moved into the repository.

I may have forgotten to mention it before, but you do indeed have write
access to the AioPlugin repository.

Dave