Hi, Thank's to test my patchs.
> I have some comments on your code before I commit: > > - In the files src/m_javaloader.cc and .h, you should add your > name to the top as one of the copyright holders. > I see you copied one of the other modules, so it's fine > to list both NetDirect and you. Done > - How big can java modules be? I suspect they can be quite large, > maybe even a few megabytes. Does it make sense to force > the library user to always load modules via the filesystem? > It probably makes sense for the majority of the time, > but so far, fopen() calls have been left to the non-library > code. For big module, I have add a FIXME in the code... Beside of, I'm testing with big module, and bjavaloader has failed :( For the fopen, I think that fopen should be made by bjavaloader tools (and not the library). So, bjavaloader sends a big buffer with all the content of file (and this file comes from filesystem, URL, etc...) But it is for a next release... > - In JavaLoader::StartStream(), you seem to do an endian byte swap > for the filesize twice. Is this required? I have add a FIXME. > - Don't hesitate to add named constants for the commands in > protocol.h or protostructs.h. I don't know always the command mean :( > - MIN_PACKET_SIZE should probably be 5 anyway... I think the correct > fix would be to remove it entirely and just check for > the required size, which in most cases where MIN is used, > is the Packet header size (which is 5). > > The idea is that library code should always check that > the data exists before accessing it. It makes more sense > to check for a specific header size than some constant minimum. > > - MAX_PACKET_SIZE needs testing... the device will fragment as it > needs, but when uploading, we can't blow the device's limit. > > It might make sense to put the fragentation limit in the > Socket class constructor, so that each mode class could > maybe set it as needed. I'll have to look into this more > closely. Comments welcome. I have splitted the data packets and the command packets. It's cleaner. > - I'd like to understand the sequence packet stuff better, rather > than turn it off and on like that. Me too :) But for a first release / test, it's quite good. I post the patch on my website to avoid the approuvement and issues :) http://www.progweb.com/modules/blackberry/ Commit these patches are usefull, onfy if somebody want to work and tests. Regards, Nicolas VIVIEN Le vendredi 09 janvier 2009 à 19:27 -0500, Chris Frey a écrit : > On Thu, Jan 08, 2009 at 08:48:52PM +0100, Nicolas wrote: > > Hi, > > > > I'm proud to announce the first patch release of my JavaLoader > > application. > > > > This patch is made with GIT. (Apply the two patchs) > > > > Then, build Barry : > > $ cd /usr/local/src/barry > > $ patch -p0 < 0001-Add-JavaLoader-tool.patch > > $ patch -p0 < 0002-Move-bjavaloader-tools.patch > > These patches aren't quite complete, and they don't include what was in > the alpha patch set. > > I suspect that our emails crossed last night... I responded to the alpha > patchset, and you sent this one. I'm very interested in your comments > on my previous email. > > Thanks! > - Chris > > > ------------------------------------------------------------------------------ > Check out the new SourceForge.net Marketplace. > It is the best place to buy or sell services for > just about anything Open Source. > http://p.sf.net/sfu/Xq1LFB > _______________________________________________ > Barry-devel mailing list > Barry-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/barry-devel ------------------------------------------------------------------------------ Check out the new SourceForge.net Marketplace. It is the best place to buy or sell services for just about anything Open Source. http://p.sf.net/sfu/Xq1LFB _______________________________________________ Barry-devel mailing list Barry-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/barry-devel
