[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Rob Savoye
Subject: Re: [Gnash-commit] /srv/bzr/gnash/trunk r12207: merge from branch all ExternalInterface refactoring.
Date: Thu, 03 Jun 2010 13:32:10 -0600
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20100430 Fedora/3.0.4-2.fc12 Lightning/1.0b1 Thunderbird/3.0.4

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 -

reply via email to

[Prev in Thread] Current Thread [Next in Thread]