Re: [Gnash-commit] /srv/bzr/gnash/trunk r12207: merge from branch all ExternalInterface refactoring.

[Rob Savoye]

On 06/03/10 10:31, Benjamin Wolsey wrote:
>> +#include <sys/ioctl.h>
>> +#include <sys/types.h>
> 
> Is there a way of removing these headers from libcore? If they aren't
> present on a particular system, we'd start needing ifdefs and we'd done
> quite well getting those out of libcore.

  We already use ioctl in other places, and adding an ifdef isn't a
problem as we already have it ifdef'd for WIN32. I wouldn't want to use
ioctls all over Gnash, but in this case it serves a legit purpose. I use
it to see how many bytes are in the network buffer (this also works on
win32), and then allocate a buffer of exactly that size when calling
read(). This simplifies things as it avoids either wasting space, or
resizing the buffers later.

> Would be better dropped. I don't think it's needed in the file currently
> anyway.

  I needed to add std:: in three place, no big deal, so I made the change.

> I think 'return "<null/>"' would be clearer and more efficient here!
        ...
> Same idea here, and in a couple of further functions.

  Got those too. These few changes are in as of revno #12215.

>> +size_t
>> +ExternalInterface::writeBrowser(int fd, const std::string &data)
>> +{
>> +    if (fd > 0) {
>> +        return ::write(fd, data.c_str(), data.size());
>> +    }
>> +
>> +    return -1;
>> +}
> 
> It might be worth making an ExternalInterface instance and storing the
> fd (if that can be done at construction of movie_root and never changes)
> to save passing it every time, but then again it might not.

  I've changed some of the code around this, as I now do pass in only
the std::string without any file descriptor. So this method can probably
go away. This way it uses movie_root::_controlfd, which was already
there, and never changes.

        - rob -