From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x242.google.com (mail-wm0-x242.google.com [IPv6:2a00:1450:400c:c09::242]) by hurricane.the-brannons.com (Postfix) with ESMTPS id 4C63E7A577 for ; Thu, 27 Jul 2017 11:21:23 -0700 (PDT) Received: by mail-wm0-x242.google.com with SMTP id 184so24707804wmo.3 for ; Thu, 27 Jul 2017 11:21:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=geoffair-info.20150623.gappssmtp.com; s=20150623; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-transfer-encoding; bh=dEWJRg8jjbg2atx8S901JPOdmX5AV6Rjdvm8NO718IQ=; b=0Z7QGthZ6EBEEH/B7gSQ5UM+XrzSHpgKDLuuUOILHJ2zJ3rd/sB3U/jhzG6SULZ14p S0OoAc3YcQivQqOOsZXNIRmmiydDPqJ99KLIGCRZgkCCc5WTIt92Pyo5HCMcaMIuakK8 7GltSP03QOzPzt60F7S3tqi/2N8vwUAAGPpkG3uuaoRfWKw72mQbw06+oeFUnuDhRt4j mIIQ4eKUhz/X5aJ7gJKX8a+33Umh/tahCSmTJQl2WxQE8SxaQTCvz7dob4CO0tfGyMX6 dPi8gboS1boZ/Ynxdk3hocEjjuyc5O7jzzLFz/WJy1fy9QuCD1ytvZLvCm3nYkJwaE/c OQjw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=dEWJRg8jjbg2atx8S901JPOdmX5AV6Rjdvm8NO718IQ=; b=MeGaZsyPcyhLMwhfXgjhPdZuhKn21W0TvlkMZEfsqRIyoxoKXKn1SlnPJ/jtm+WLj5 W6QiNvrHW+0l0dA+0eo9phnVICwFJzBqS4+vYETTWDf2TRQI5SD6xB6cVVWZbTGZCKfd yG5TD6t6zPwZDIyxWxHWkIHSbPHDWc1dsYS9i8G7ohFIsIWsV9/D98HE7xNTKqiKoHRC c4dmbLL3DMafoLtxttwibupCyk/ELQP9fASnFYUHriBye6kk0AODEqgzTGJ7xoslh/K1 nU1EpRH2xd+b84i9FYzAYDW/d8vob0rEO0GH4S3JjPDOKIBtjJEngAoZO+kzHZXYj04G CFCA== X-Gm-Message-State: AIVw1116QxS1naARvsBgpnaFMlgGEhnijSr/r8FRaupsUUQQhkHaNpJH xHxDcJEz2t03Rfgl X-Received: by 10.28.71.5 with SMTP id u5mr4242747wma.138.1501179702319; Thu, 27 Jul 2017 11:21:42 -0700 (PDT) Received: from ?IPv6:2a01:cb04:4ba:c500:c165:64af:38ae:c37b? (2a01cb0404bac500c16564af38aec37b.ipv6.abo.wanadoo.fr. [2a01:cb04:4ba:c500:c165:64af:38ae:c37b]) by smtp.googlemail.com with ESMTPSA id l68sm25540435wrc.12.2017.07.27.11.21.41 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 27 Jul 2017 11:21:41 -0700 (PDT) To: Karl Dahlke , Edbrowse-dev@lists.the-brannons.com, Chris Brannon References: <20170625230859.eklhad@comcast.net> From: Geoff McLane Message-ID: <90686f79-6a2c-ef0f-be56-8937b2f66a8a@geoffair.info> Date: Thu, 27 Jul 2017 20:21:40 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <20170625230859.eklhad@comcast.net> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Edbrowse-dev] -Wall X-BeenThere: edbrowse-dev@lists.the-brannons.com X-Mailman-Version: 2.1.24 Precedence: list List-Id: Edbrowse Development List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Jul 2017 18:21:23 -0000 Hi Karl, Chris, Thanks for the heads up about the master branch... with a few minor changes it compiles and passes the jsrt test... Re: Unix Well the cmake build has always been adding -Wall, but then suppressed some warning... I have just removed those suppressions, as one should do now and then to test, and it seems there are no current warning... Re: Windows I guess the equivalent in Windows is the /W3, which is added automatically by cmake... But again I had been suppressing a number of warnings... There is a /W4, but do not suggest using this... it seem /W3 is very sufficient... Removing that suppression and MSVC14 2015 shows 736 warnings, no errors... See full list - http://geoffair.org/tmp/bldlog-w.txt but be aware, each is listed twice. Once for the Debug build, and once for the Release build... One form of very common C4996 warning is this - ...\main.c(372): warning C4996: 'getenv': This function or variable may be unsafe. Consider using _dupenv_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. Most of the time I see no particular reason to adopt a non-portable replacement, thus adding the suggested macro _CRT_SECURE_NO_WARNINGS immediately reduces the warnings to 393... See http://geoffair.org/tmp/bldlog-w1.txt, but again be aware of the Debug/Release duplicates... But this leaves some other warning C4996, of the form - ...\main.c(492): warning C4996: 'close': The POSIX name for this item is depreciated. Instead, use the ISO C and C++ conformant name: _close. See online help for details. From discussion in several other project, it seems unix people reject this MS claim... and I tend to agree... It can be fixed by adding the following in a common include file - #ifdef _MSC_VER #define close _close #endif Note this is just for a MS MSVC build, so use _MSC_VER rather than the DOSLIKE macro you have... But this is a list of functions like open, chmod, creat, write, unlink, getcwd, etc... Or suppressing them, as I have done in CMakeLists.txt if (WIN32) if (MSVC) set(WARNING_FLAGS "${WARNING_FLAGS} /wd4996") This reduces the warning to just 283. That is removed some 453 warnings to date... see http://geoffair.org/tmp/bldlog-w2.txt The next most common warning is C4267 - ...\main.c(96): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data This happen in the Windows 64-bit build because a size_t is 8 bytes, while an int remains 4 bytes... But in reality, an int is large enough to hold a very big number, thus very very unlikely this would represent loss of data, but could... This can be fixed by telling the compiler you know by a static cast, like in this case - int rlen = (int)strlen(reply); Or suppressing it - if (WIN32) if (MSVC) foreach(warning 4267) set(WARNING_FLAGS "${WARNING_FLAGS} /wd${warning}") endforeach() This reduced the warnings to 142. ie removed some 141 more... see - http://geoffair.org/tmp/bldlog-w3.txt The next group of warnings are similar - suggest possible loss of data, like - ..\main.c(176): warning C4244: '=': conversion from '__int64' to 'int', possible loss of data But some do raise suspicions, like - ..\main.c(1478): warning C4244: '=': conversion from 'long' to 'unsigned char', possible loss of data The code here is - td->key1 = strtol(v,&v, 10); Now I have not studied the code carefully, but if this represents for example a table td columns then probably a maximum of 255 is ok... But as you point out, sometimes fixing the code with a static cast makes it clear to the compiler, and to other readers of the code that you have thought of this, and accept the risk... And in this case the code following compares the value to an 'int', td->ncols... why not make the key1 and key2 ints also... But for windows, added this 4244 to the suppression list... this reduces the warnings to just 26 - see http://geoffair.org/tmp/bldlog-w4.txt The next most common is an unreferenced variable... ...\buffers.c(414): warning C4101: 'channels': unreferenced local variable Now usually gcc also warns of this, so this is usually, like in this case, in a DOSLIKE macro switch... so this, and other like cases could be fixed by - #ifndef DOSLIKE fd_set channels; #endif // !DOSLIKE But I do not consider this bad coding when there are macro switches involved, and maybe the extra works to switch each of these is not worth the effort... So I have added 4101 to the suppression list... this reduces the warnings to just 12 - see http://geoffair.org/tmp/bldlog-w5.txt Another minor item that MSVC flags is C4090, like - ...\main.c(832): warning C4090: 'function': different 'const' Again, I do not consider this important... it could be 'fixed', but choose to just add 4090 to the suppression list... And we are down to just 7 warnings - see http://geoffair.org/tmp/bldlog-w6.txt Five (5) of these are of the form - ...\sendmail.c(669): warning C4013: 'asprintf' undefined; assuming extern returning int Since this usually involves adding a header to the source, I do not like suppressing these... and since the link resolves them with no problem, maybe they can stay... One (1) is - ...\jseng-duk.c(954): warning C4018: '>': signed/unsigned mismatch Since this is in a new module, perhaps it could be fixed, instead of suppression... The last is - ..\vsprtf.c(24): warning C4005: 'va_copy': macro redefinition Since this is in a Win32 only module, and is just due to a later version of _MSC_VER have provided a fix... Have pushed 4 commits addressing the above... Regards, Geoff.