On Mon 29 Sep 2014, Nagel, Bill wrote: > This patch implements the CIFS dialect of SMB.
Whilst I've taken a look at your SMTP patch I'm probably not going to be able to merge and test this one myself as I'm a Windows engineer, however, I do have some initial feedback: * I would recommend you split the patch up into manageable stages - such as the modifications to configure.ac, the introduction of the SMB defines, the main SMB files, then integration into the rest of curl/libcurl. * Use git patches rather than diff's as it will make merging a lot earier for us - meaning less time to get your work into the repository ;-) * Have you prepared any tests that can be added to our test suite? * Have you prepared any documentation for SMB? For example in the curl man page, the URL section of the libcurl documentation, any curl_easy_setopt() that are affected by the new protocol, etc... * There are a number of instances that I saw from scanning the code that don't adhere to our coding standard - 1) braces around single line if statements are not needed 2) Action parts of a single line if statement are not on a separate line and currently on the same line as the condition * I would personally prefer that you don't use assignment operators in if statements * How does this addition affect build platforms that don't use autoconf - for example Windows? Is CURL_DISABLE_SMB defined for such platforms / does it need to be? Kind Regards Steve ------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.html
