[Box Backup-dev] Reviewing code -- help!

[Ben Summers]

OK then... I hope I'm not being too picky here... basically it looks  
really good and I just want to have a really high quality release.


svn diff http://bbdev.fluffy.co.uk/svn/box/chris/win32/merge/00- 
martins-original-autoconf http://bbdev.fluffy.co.uk/svn/box/chris/ 
win32/merge/06-code-cleanups/

config.guess, config.sub -- is this necessary?

lib/win32/win32test.cpp
	-- wrong place, create test/win32

lib/win32/win32_build_on_linux_using_mingw.txt
	-- move to docs/backup ?

lib/win32/win32.bat
	-- needs to move to bin/bbackupd/win32

lib/win32/WinNamedPipeStream.cpp
	-- WinNamedPipeStream::Accept(), Connect() doesn't use Name argument.
	-- define L"\\\\.\\pipe\\boxbackup" in BoxPortsAndFiles.h?
	-- WinNamedPipeStream::GetPeerCredentials() doesn't need any of the  
UNIX code

lib/server/SocketStream.h
	-- mods belong in Socket.h

bin/bbackupd/ServiceBackupDaemon.*
	-- rename to Win32ServiceBackupDaemon.* ? or move to bbackupd.cpp?

bin/bbackupd/bbwinservice.*
	-- rename to Win32bbackupdService.*


Thanks for all your hard work.

Ben



On 8 Dec 2005, at 17:41, Chris Wilson wrote:

> Hi Ben,
>
>>> >  WinNamedPipe should be an IOStream derived class, then use >   
>>> IOStreamGetLine not PipeGetLine.
>
> Now done. At least, it compiles with MinGW, not tested yet (and  
> still compiles on Linux).
>
>>> >  WinNamedPipe::Write may not work in edge cases where it  
>>> doesn't write >  all the bytes given.
>
> According to MSDN, if I read it correctly, WriteFile will not do  
> partial writes, but block until it's written everything (or an  
> error occurs). There's no way to get the number of bytes written  
> back from it.
>
> [http://msdn.microsoft.com/library/default.asp?url=/library/en-us/ 
> fileio/fs/writefile.asp]
>
>>> >  The .bat, config and installer files should not be in lib/ 
>>> win32 as >  they're not generic win32 stuff. I suggest bin/ 
>>> bbackupd/win32
>
> Ok, done.
>
>> They're not really documents either. Surely the build system will  
>> be packaging them up? Perhaps they could be included in the parcel  
>> making script?
>
> Done as well, with some small changes to makeparcels.pl to allow  
> including files ONLY on some platforms.
>
>> I think some of the structure definitions in emu.h must have come  
>> from somewhere.
>
> Sorry, not my department. You need to go to Information Retrieval.  
> </brazil>
>
>>> >  Not keen on the global and member function names beginning  
>>> with lower >  case letters (and other style "violations").
>
> OK, I think I've taken care of the thread method in BackupDaemon  
> and the WinNamedPipe (now called WinNamedPipeStream).
>
>> I think this was in the threading code. Maybe I was imagining it,  
>> there's a lot there.
>
> There were quite a few global functions and variables in emu.cpp,  
> bbwinservice.cpp and ServiceBackupDaemon.cpp with such names. I've  
> fixed all of those that I can see and which I don't think are  
> emulations of system library functions.
>
> I don't know of any others, but please point them out if you want  
> me to fix them.
>
>>> >  -AX_CHECK_MOUNT_POINT(, [AC_MSG_ERROR([[cannot work out how to  
>>> discover >  mount points on your platform]])])
>>> >  +AX_CHECK_MOUNT_POINT([],[])
>>> > >  I'm not an autoconf sort of person, but that looks worrying  
>>> to me? But >  otherwise there's only minor things.
>>>  If you mean the fact that I disabled the fatal error - Win32 has  
>>> no mount  points, as far as I know, so if autoconf bombs out then  
>>> you can't build.
>>
>> But doesn't that break other platforms?
>
> As Martin pointed out, all it means (I think) is that Box now  
> configures on platforms where statfs does not return a f_mntonname.  
> With the latest changes in 06-code-cleanups, Win32 now uses Nick's  
> statfs function again, and platforms without f_mntonname will try  
> to work it out from mtab.
>
>>>  I couldn't see an easy way at the time to disable this check  
>>> just for  Win32, although I'm learning and I have some ideas now.
>>>  If you mean the empty strings [], then I don't know. I thought  
>>> it looked  better than entirely missing arguments. However, I  
>>> don't know much about  Autoconf's quoting rules, perhaps someone  
>>> can correct me.
>>
>> I hope Martin will come up with the answer.
>
> I've disabled the check and removed the GPL code from SVN. Now I  
> just don't even check for nanosleep on Win32, as we never call it  
> or even compile the code that uses it.
>
> Cheers, Chris.
> -- 
> _ ___ __     _
>  / __/ / ,__(_)_  | Chris Wilson <0000 at qwirx.com> - Cambs UK |
> / (_/ ,\/ _/ /_ \ | Security/C/C++/Java/Perl/SQL/HTML Developer |
> \ _/_/_/_//_/___/ | We are GNU-free your mind-and your software |
>
> _______________________________________________
> Boxbackup-dev mailing list
> Boxbackup-dev at fluffy.co.uk
> http://lists.warhead.org.uk/mailman/listinfo/boxbackup-dev