Re: [Bug-wget] [Bug-Wget] It's spring cleaning time!
Darshit Shah writes: > Not too hard. > > Here it is. Thank you both, pushed now! Regards, Giuseppe
Re: [Bug-wget] [Bug-Wget] It's spring cleaning time!
Am Sonntag, 11. Mai 2014, 15:56:15 schrieb Giuseppe Scrivano:
> Darshit Shah writes:
> > Subject: [PATCH] Fix LOTS of compiler warnings
>
> great work! Just some minor comments:
> > * http.c: Fix small memory leak
> >
> > diff --git a/src/css-url.c b/src/css-url.c
> > index f97690d..51e43b4 100644
> > --- a/src/css-url.c
> > +++ b/src/css-url.c
> > @@ -64,8 +64,8 @@ typedef struct yy_buffer_state *YY_BUFFER_STATE;
> >
> > extern YY_BUFFER_STATE yy_scan_bytes (const char *bytes,int len );
> > extern int yylex (void);
> >
> > -#if 1
> > -const char *token_names[] = {
> > +#if 0
> > +static const char *token_names[] = {
> >
> >"CSSEOF",
> >"S",
> >"CDO",
>
> if this is not needed, what about just dropping it? I would prefer we
> keep code that is used somewhere.
Then let's drop it.
Due to version control, the code is not really lost... (if someone minds to
use it again).
>
> > -#define NTLMFLAG_NEGOTIATE_DOMAIN_SUPPLIED (1<<12)
> > -#define NTLMFLAG_NEGOTIATE_WORKSTATION_SUPPLIED (1<<13)
> > -#define NTLMFLAG_NEGOTIATE_LOCAL_CALL(1<<14)
> > -#define NTLMFLAG_NEGOTIATE_ALWAYS_SIGN (1<<15)
> > -#define NTLMFLAG_TARGET_TYPE_DOMAIN (1<<16)
> > -#define NTLMFLAG_TARGET_TYPE_SERVER (1<<17)
> > -#define NTLMFLAG_TARGET_TYPE_SHARE (1<<18)
> > -#define NTLMFLAG_NEGOTIATE_NTLM2_KEY (1<<19)
> > -#define NTLMFLAG_REQUEST_INIT_RESPONSE (1<<20)
> > -#define NTLMFLAG_REQUEST_ACCEPT_RESPONSE (1<<21)
> > -#define NTLMFLAG_REQUEST_NONNT_SESSION_KEY (1<<22)
> > -#define NTLMFLAG_NEGOTIATE_TARGET_INFO (1<<23)
> > +/* #define NTLMFLAG_NEGOTIATE_DOMAIN_SUPPLIED (1<<12) */
> > +/* #define NTLMFLAG_NEGOTIATE_WORKSTATION_SUPPLIED (1<<13) */
> > +/* #define NTLMFLAG_NEGOTIATE_LOCAL_CALL(1<<14) */
> > +/* #define NTLMFLAG_NEGOTIATE_ALWAYS_SIGN (1<<15) */
> > +/* #define NTLMFLAG_TARGET_TYPE_DOMAIN (1<<16) */
> > +/* #define NTLMFLAG_TARGET_TYPE_SERVER (1<<17) */
> > +/* #define NTLMFLAG_TARGET_TYPE_SHARE (1<<18) */
> > +/* #define NTLMFLAG_NEGOTIATE_NTLM2_KEY (1<<19) */
> > +/* #define NTLMFLAG_REQUEST_INIT_RESPONSE (1<<20) */
> > +/* #define NTLMFLAG_REQUEST_ACCEPT_RESPONSE (1<<21) */
> > +/* #define NTLMFLAG_REQUEST_NONNT_SESSION_KEY (1<<22) */
> > +/* #define NTLMFLAG_NEGOTIATE_TARGET_INFO (1<<23) */
>
> same here, what about just dropping them?
Yes, same here.
Darshit, I guess it would be easiest if you change the patch !?
Tim
Re: [Bug-wget] [Bug-Wget] It's spring cleaning time!
Darshit Shah writes:
> Subject: [PATCH] Fix LOTS of compiler warnings
great work! Just some minor comments:
> * http.c: Fix small memory leak
> diff --git a/src/css-url.c b/src/css-url.c
> index f97690d..51e43b4 100644
> --- a/src/css-url.c
> +++ b/src/css-url.c
> @@ -64,8 +64,8 @@ typedef struct yy_buffer_state *YY_BUFFER_STATE;
> extern YY_BUFFER_STATE yy_scan_bytes (const char *bytes,int len );
> extern int yylex (void);
>
> -#if 1
> -const char *token_names[] = {
> +#if 0
> +static const char *token_names[] = {
>"CSSEOF",
>"S",
>"CDO",
if this is not needed, what about just dropping it? I would prefer we
keep code that is used somewhere.
> -#define NTLMFLAG_NEGOTIATE_DOMAIN_SUPPLIED (1<<12)
> -#define NTLMFLAG_NEGOTIATE_WORKSTATION_SUPPLIED (1<<13)
> -#define NTLMFLAG_NEGOTIATE_LOCAL_CALL(1<<14)
> -#define NTLMFLAG_NEGOTIATE_ALWAYS_SIGN (1<<15)
> -#define NTLMFLAG_TARGET_TYPE_DOMAIN (1<<16)
> -#define NTLMFLAG_TARGET_TYPE_SERVER (1<<17)
> -#define NTLMFLAG_TARGET_TYPE_SHARE (1<<18)
> -#define NTLMFLAG_NEGOTIATE_NTLM2_KEY (1<<19)
> -#define NTLMFLAG_REQUEST_INIT_RESPONSE (1<<20)
> -#define NTLMFLAG_REQUEST_ACCEPT_RESPONSE (1<<21)
> -#define NTLMFLAG_REQUEST_NONNT_SESSION_KEY (1<<22)
> -#define NTLMFLAG_NEGOTIATE_TARGET_INFO (1<<23)
> +/* #define NTLMFLAG_NEGOTIATE_DOMAIN_SUPPLIED (1<<12) */
> +/* #define NTLMFLAG_NEGOTIATE_WORKSTATION_SUPPLIED (1<<13) */
> +/* #define NTLMFLAG_NEGOTIATE_LOCAL_CALL(1<<14) */
> +/* #define NTLMFLAG_NEGOTIATE_ALWAYS_SIGN (1<<15) */
> +/* #define NTLMFLAG_TARGET_TYPE_DOMAIN (1<<16) */
> +/* #define NTLMFLAG_TARGET_TYPE_SERVER (1<<17) */
> +/* #define NTLMFLAG_TARGET_TYPE_SHARE (1<<18) */
> +/* #define NTLMFLAG_NEGOTIATE_NTLM2_KEY (1<<19) */
> +/* #define NTLMFLAG_REQUEST_INIT_RESPONSE (1<<20) */
> +/* #define NTLMFLAG_REQUEST_ACCEPT_RESPONSE (1<<21) */
> +/* #define NTLMFLAG_REQUEST_NONNT_SESSION_KEY (1<<22) */
> +/* #define NTLMFLAG_NEGOTIATE_TARGET_INFO (1<<23) */
same here, what about just dropping them?
Giuseppe
Re: [Bug-wget] [Bug-Wget] It's spring cleaning time!
On Saturday 03 May 2014 21:18:27 Darshit Shah wrote: > Hello again, > > This took too long. I had the patches, but completely forgot to send them > around. The attached patch was submitted in multiple commits by Tim, > > @Tim: I've taken the liberty to accumulate all the patches into one and > write a cumulative ChangeLog entry. It's HUGE. Do, check it out to see if > it's fine. Hi Darshit, not really HUGE, and those changes are trivial. Could you please change C99 comments into C89 comments in src/http-ntlm.c ? I forgot Wget's C89 subservience... ;-) Thank you. Tim
Re: [Bug-wget] [Bug-Wget] It's spring cleaning time!
On Mon, May 5, 2014 at 9:40 AM, Tim Ruehsen wrote: > On Saturday 03 May 2014 21:18:27 Darshit Shah wrote: > > Hello again, > > > > This took too long. I had the patches, but completely forgot to send them > > around. The attached patch was submitted in multiple commits by Tim, > > > > @Tim: I've taken the liberty to accumulate all the patches into one and > > write a cumulative ChangeLog entry. It's HUGE. Do, check it out to see if > > it's fine. > > Hi Darshit, > > not really HUGE, and those changes are trivial. > > I found the ChangeLog entry huge. It was so long. It's mostly trivial, yes. Wish I had a script to write the ChangeLogs. :) > Could you please change C99 comments into C89 comments in src/http-ntlm.c ? > I forgot Wget's C89 subservience... ;-) > I'll fix that once I'm back on my own machine. > > Thank you. > > Tim > -- Thanking You, Darshit Shah
Re: [Bug-wget] [Bug-Wget] It's spring cleaning time!
On Sun, Mar 30, 2014 at 9:35 PM, Tim Rühsen wrote: > Am Sonntag, 30. März 2014, 16:00:07 schrieb Darshit Shah: >> Hello, >> >> I've been wanting to clean up the code for Wget for some time now. >> Today, I wrote a small script that compiles Wget with a bunch of >> warning flags and uploads each warning to GitHub as an issue. >> >> These issues have been created against the fork of Wget that I >> maintain on https://github.com/darnir/wget >> >> As of now, I compiled Wget using Clang and CFLAGS="-Wextra -Wall >> -pedantic -std=gnu89" >> >> These flags end up spewing around 60 warnings, all of them semantic >> issues. The issues have been labeled based on the flag that caused >> them and the type of issue it is according to clang. >> >> I wanted to run clang with -Weverything, but that throws nearly 800+ >> warnings. Hence, I figured, let's clean this up first and we can then >> begin looking at the other warnings. Some of them are probably false >> positives, but tohers seem to have some more subtle problems with >> platform independence. > > I like the idea (always liked it) to fix all these warnings - I guess some of > my patches were lost... > Aah! I've been working on some cleaning too. Sadly, I ran a `git reset --hard origin/master` too quickly. Before I could even commit the changes. Now, those are lost. I'll work on them again. > However you define "false positive", they simply pollute the screen and when > searching for real issues, you have to read through again and again. In this > meaning, there is no false positive - except the compiler has a bug (the clang > analyzer really finds some (cough) interesting things). > Yes. Clang is still under development and finds some crazy things. I'll try and have the script work with GCC. I chose clang because its my default compiler and also, I plan on hooking into libclang at some point, so the script doesn't have to parse things. > I once made a patch that fixed those 'const' warnings when compiling with - > DDEBUG (triggers selftest code). But I think it was not applied :-( > If you have the patch, please do send it again! > However, good idea, and I try to find some time ! Thanks.. I'll try and fix a few things in the meanwhile. > > Tim -- Thanking You, Darshit Shah
Re: [Bug-wget] [Bug-Wget] It's spring cleaning time!
Am Sonntag, 30. März 2014, 16:00:07 schrieb Darshit Shah: > Hello, > > I've been wanting to clean up the code for Wget for some time now. > Today, I wrote a small script that compiles Wget with a bunch of > warning flags and uploads each warning to GitHub as an issue. > > These issues have been created against the fork of Wget that I > maintain on https://github.com/darnir/wget > > As of now, I compiled Wget using Clang and CFLAGS="-Wextra -Wall > -pedantic -std=gnu89" > > These flags end up spewing around 60 warnings, all of them semantic > issues. The issues have been labeled based on the flag that caused > them and the type of issue it is according to clang. > > I wanted to run clang with -Weverything, but that throws nearly 800+ > warnings. Hence, I figured, let's clean this up first and we can then > begin looking at the other warnings. Some of them are probably false > positives, but tohers seem to have some more subtle problems with > platform independence. I like the idea (always liked it) to fix all these warnings - I guess some of my patches were lost... However you define "false positive", they simply pollute the screen and when searching for real issues, you have to read through again and again. In this meaning, there is no false positive - except the compiler has a bug (the clang analyzer really finds some (cough) interesting things). I once made a patch that fixed those 'const' warnings when compiling with - DDEBUG (triggers selftest code). But I think it was not applied :-( However, good idea, and I try to find some time ! Tim signature.asc Description: This is a digitally signed message part.
[Bug-wget] [Bug-Wget] It's spring cleaning time!
Hello, I've been wanting to clean up the code for Wget for some time now. Today, I wrote a small script that compiles Wget with a bunch of warning flags and uploads each warning to GitHub as an issue. These issues have been created against the fork of Wget that I maintain on https://github.com/darnir/wget As of now, I compiled Wget using Clang and CFLAGS="-Wextra -Wall -pedantic -std=gnu89" These flags end up spewing around 60 warnings, all of them semantic issues. The issues have been labeled based on the flag that caused them and the type of issue it is according to clang. I wanted to run clang with -Weverything, but that throws nearly 800+ warnings. Hence, I figured, let's clean this up first and we can then begin looking at the other warnings. Some of them are probably false positives, but tohers seem to have some more subtle problems with platform independence. This is a kind request to everyone here, please look at the issue list, claim something if you're working on it and fix it. I'll enable more warning flags as these get fixed. I would also suggest a spring cleaning day, sometime next week maybe when we can collectively try and clean as much of the code as possible. The scripts I wrote to generate this issue list will be publicly available as soon as I manage to clean them up. They're still a horrible hack, but it works. -- Thanking You, Darshit Shah
