<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2015-04-02 3:56 GMT+02:00 David T. Lewis <span dir="ltr"><<a href="mailto:lewis@mail.msen.com" target="_blank">lewis@mail.msen.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class=""><br>
On Wed, Apr 01, 2015 at 10:16:37PM +0200, Nicolas Cellier wrote:<br>
><br>
> Hello,<br>
> I noticed a few #positive32BitValueOf: usage in plugins for the purpose of<br>
> retrieving a pointer stored in a #sqInt or #usqInt.<br>
<br>
</span>That would definitely be wrong. Any plugin that stores machine pointers in<br>
sqInt or usqInt is broken.<br>
<br>
There are number of plugins that are still not updated for 64-bit VMs. I've<br>
tried to track some of the status of this on <a href="http://bugs.squeak.org" target="_blank">bugs.squeak.org</a> although the<br>
information is not complete.<br>
<br>
Of the top of my head, I think that the SSL plugin, Camera plugin, anything<br>
dependent on SurfacePlugin, and probably a few others are broken. FFIPlugin<br>
was notably broken (with the fixes are on Mantis but not integrated), though<br>
that my no longer be relevant with newer FFI plugins.<br>
<span class=""><br>
><br>
> Yes, a #sqInt is large enough to hold a pointer, but shouldn't it be filled<br>
> with positiveMachineIntegerValueOf:?<br>
> At least this is necessary for spur64.<br>
<br>
</span>A sqInt is *not* large enough to hold a pointer, specifically in the most<br>
common case of a 32-bit object memory running on a 64-bit interpreter VM.<br>
<span class=""><br></span></blockquote><br><div><span class=""><br></span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">
><br>
> Here are 3 patches attached.<br>
><br>
> Nicolas<br>
<br>
</span>Thanks! I know your patches are focused on Spur but I'll try running them also on<br>
the interpreter VM as soon as I am able. Note that positiveMachineIntegerValueOf:<br>
is a Spur idiom, although I would not expect that it would necessary to rely on that,<br>
since usually just declaring the variables correctly is sufficient. So for example in<br>
QuicktimePlugin>>primitiveSetGWorldPtrOntoSurface:width:height:rowBytes:depth:movie:<br>
there is a variable #buffer that looks like it needs to be a (char *), but is undeclared<br>
and takes the default #sqInt. Casting it to (char *) will always fail in the case of<br>
a 32-bit sqInt on a 64-bit host platform, regardless of which idiom you use in the<br>
cCoerce cast. It is better to declare both #buffer and #bitMapPtr explicitly, and<br>
get rid of the cast. That usually requires also fixing up the declarations in the<br>
support code and header files in ./Cross, but that's probably the only way to get<br>
it right.<br>
<br>
Dave<br>
<br></blockquote><div><br></div><div></div><div>Let me reformulate.<br><br>My patch is for the case when we stored an external pointer (a handle or something) into an Integer at image side (possibly a LargeInteger).<br><br></div><div>When the image is willing to pass the handle back to another primitive of the plugin, the conversion from smalltalk Small/LargeInteger -> pointer has to occur one way or another.<br><br></div><div>As I see it, something as positiveMachineIntegerValueOf: that would use either 32 or 64 bits variant depending on wordSize is the way to go.<br><br></div><div>What you say, is that the variable that holds the pointer should not be declared #sqInt, because even if it works with Spur64, it's not large enough in classical Interpreter.<br><div><br></div><div></div><div>The classical Interpreter version of <span class="">positiveMachineIntegerValueOf: could answer an unsigned long for example (asserting that (sizeof(long)>=wordSize)), so we can continue using that message for the compatibility layer (having single plugin variant for several VM brands).<br><br></span></div></div><div>But you are right, the plugin itself should not use a #sqInt declaration for holding the handle/pointer - even if it works with Spur64, I'll try to make another pass with that in mind.<br><br></div><div>Nicolas<br></div></div><br></div></div>