Re: [Bug-wget] [PATCH] Regression since wget 1.10: no_prefix function is *bad*
Am Donnerstag, 16. Mai 2013 schrieb Giuseppe Scrivano: > Tim Rühsen writes: > > > Hi Alex, > > > > yes, it is the _PC_NAME_MAX issue which is only valid for pathconf(). > > > > Attached is the little patch to fix it. > > > > Since MinGW is based on gcc-4.6, C99 should be available. > > As the Wiki states (well, the entry is 3 years old...), printf() is the main > > issue. But there may be some other functions that don't behave C99 compliant. > > Sound not like a compiler, but a library issue. > > > > Maybe some functions have to be provided by Wget. If we just had a list of > > issues/functions... > > most of these portability problems should be fixed by gnulib. Have you > checked those? I am pretty sure that gnulib provides a POSIX compliant > printf replacement on platforms that lack it. Now, I checked the gnulib sources. And as far as I can see, the printf like functions are c99 compliant. So we have a C99 (cross-) compiler plus a C99 function library (for Win32, Win64 and MSDOS). Are there any OSes that doesn't have a c99 compiler and that Wget needs/wants to provide support for ? Background: As many C programmers I am used to C99 and for me it is a PITA to check my patches to be 100% C89 compliant before sending them. It is not fun. So I wish to see C99 acceptance in Wget in the future... Regards, Tim
Re: [Bug-wget] [PATCH] Regression since wget 1.10: no_prefix function is *bad*
Tim Rühsen writes: > Hi Alex, > > yes, it is the _PC_NAME_MAX issue which is only valid for pathconf(). > > Attached is the little patch to fix it. > > Since MinGW is based on gcc-4.6, C99 should be available. > As the Wiki states (well, the entry is 3 years old...), printf() is the main > issue. But there may be some other functions that don't behave C99 compliant. > Sound not like a compiler, but a library issue. > > Maybe some functions have to be provided by Wget. If we just had a list of > issues/functions... most of these portability problems should be fixed by gnulib. Have you checked those? I am pretty sure that gnulib provides a POSIX compliant printf replacement on platforms that lack it. -- Giuseppe
Re: [Bug-wget] [PATCH] Regression since wget 1.10: no_prefix function is *bad*
Hi Alex, snprintf %a seems to print the correct result with wine (set to WinXP), but the same executable on a real WinXP just prints 'a'. Replacing the sprintf() by __mingw_sprintf printed the correct result with wine and on the WinXP machine. > About C99 - sorry, i think "If article isn't changed/removed then it's > still actually". Yes, in MinGW mailing lists i find mention about > "-std=c99" and "changes to mingw_snprintf". Sorry for mistake. > May be it's time for new test branch - "wget-C99"? And then find what > fragments (isn't work)/(cannot be hooked) in MinGW/CygWin/MSVC and need to > replace? Something like "From practice to theory". That is a good idea. Should Guiseppe create a test branch upstream or how can we go on ? > > how do I make a diff to some commit back in time or to upstream ? > From current to upstream > git format-patch origin/master > > From current to some commit > git format-patch ccd369d > > Single commit > git format-patch ccd369d^! > > Between two commits > git format-patch 027d9f...ae80fd2 Thanks. Tim
Re: [Bug-wget] [PATCH] Regression since wget 1.10: no_prefix function is *bad*
Greetings, Tim Rühsen About C99 - sorry, i think "If article isn't changed/removed then it's still actually". Yes, in MinGW mailing lists i find mention about "-std=c99" and "changes to mingw_snprintf". Sorry for mistake. May be it's time for new test branch - "wget-C99"? And then find what fragments (isn't work)/(cannot be hooked) in MinGW/CygWin/MSVC and need to replace? Something like "From practice to theory". how do I make a diff to some commit back in time or to upstream ? From current to upstream git format-patch origin/master From current to some commit git format-patch ccd369d Single commit git format-patch ccd369d^! Between two commits git format-patch 027d9f...ae80fd2 -- Best regars, Alex Hi Alex, yes, it is the _PC_NAME_MAX issue which is only valid for pathconf(). Attached is the little patch to fix it. Since MinGW is based on gcc-4.6, C99 should be available. As the Wiki states (well, the entry is 3 years old...), printf() is the mainissue. But there may be some other functions that don't behave C99 compliant. Sound not like a compiler, but a library issue. Maybe some functions have to be provided by Wget. If we just had a list ofissues/functions... Regards, Tim
Re: [Bug-wget] [PATCH] Regression since wget 1.10: no_prefix function is *bad*
Hi Alex,
yes, it is the _PC_NAME_MAX issue which is only valid for pathconf().
Attached is the little patch to fix it.
Since MinGW is based on gcc-4.6, C99 should be available.
As the Wiki states (well, the entry is 3 years old...), printf() is the main
issue. But there may be some other functions that don't behave C99 compliant.
Sound not like a compiler, but a library issue.
Maybe some functions have to be provided by Wget. If we just had a list of
issues/functions...
Regards, Tim
Am Montag, 13. Mai 2013 schrieb Bykov Aleksey:
> Greetings, Tim Rühsen.
> Possible that i'm understood wrong, but according to
> http://www.mingw.org/wiki/C99 MinGW doesn't support C99 at all. So i'm not
> sure about cross-compile.
> May be after implementation of C99 code it can be compiled only through
> CygWin (don't remember exactly, but two years ago Wget compiled at CygWin
> required on pure Windows only several libraries as dependencies. If need,
> i'll try to check this at week).
>
> P.S. Currently Wget compiled without any problem in Windows MinGW and
> resulting binary work in pure Windows without any dependencies.
>
> P.P.S. What "minor compile issue in url.c"? I get only error with
> undefined PC_NAME_MAX (just add it to compiler flags)?
>
> P.P.P.S. Sorry for bad English.
>
> Best regards, Alex.
>
> > Thanks for the hint. I just installed Debians MinGW packages and it
> > workedlike a charm (except a minor compile issue in url.c).
> >
> > For anyone who wants to try:
> > $ make distclean
> > $ ./configure --host=i686-w64-mingw32 --without-ssl --disable-ipv6
> > $ make
> > $ wine src/wget.exe http://www.example.com
> >
> > I have no real Windows installation around to test wget.exe.
> > But assuming, it works: Is there any need to stick at c89 code ?
> > Or in other words: do we still have to support native Windows
> > compilation withMSVC (couldn't these people install and use mingw) ?
> >
> > Regards, Tim
>
From d540fd5dbd3644936a8ad1a384516abba10de268 Mon Sep 17 00:00:00 2001
From: Tim Ruehsen
Date: Thu, 9 May 2013 19:53:36 +0200
Subject: [PATCH 1/3] src/utils.c cleanup
---
src/ChangeLog |6 ++
src/utils.c | 66 -
2 files changed, 29 insertions(+), 43 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index f4fa342..84a9645 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,9 @@
+2013-05-09 Tim Ruehsen
+
+ * utils.c: use standard string functions instead of self-written
+ code in acceptable(), match_tail(), suffix(), has_wildcards_p().
+ Avoid some warnings in test code.
+
2013-05-05 mancha (tiny change)
* gnutls.c (ssl_connect_wget): Don't abort on non-fatal alerts
diff --git a/src/utils.c b/src/utils.c
index faae62e..f7baed6 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -900,15 +900,14 @@ static bool in_acclist (const char *const *, const char *, bool);
bool
acceptable (const char *s)
{
- int l = strlen (s);
+ const char *p;
if (opt.output_document && strcmp (s, opt.output_document) == 0)
return true;
- while (l && s[l] != '/')
---l;
- if (s[l] == '/')
-s += (l + 1);
+ if ((p = strrchr(s, '/')))
+s = p + 1;
+
if (opt.accepts)
{
if (opt.rejects)
@@ -919,6 +918,7 @@ acceptable (const char *s)
}
else if (opt.rejects)
return !in_acclist ((const char *const *)opt.rejects, s, true);
+
return true;
}
@@ -1018,29 +1018,15 @@ accdir (const char *directory)
bool
match_tail (const char *string, const char *tail, bool fold_case)
{
- int i, j;
+ int pos = strlen (string) - strlen(tail);
- /* We want this to be fast, so we code two loops, one with
- case-folding, one without. */
+ if (pos < 0)
+ return false; /* tail is longer than string */
if (!fold_case)
-{
- for (i = strlen (string), j = strlen (tail); i >= 0 && j >= 0; i--, j--)
-if (string[i] != tail[j])
- break;
-}
- else
-{
- for (i = strlen (string), j = strlen (tail); i >= 0 && j >= 0; i--, j--)
-if (c_tolower (string[i]) != c_tolower (tail[j]))
- break;
-}
-
- /* If the tail was exhausted, the match was succesful. */
- if (j == -1)
-return true;
+return strcmp (string + pos, tail);
else
-return false;
+return strcasecmp (string + pos, tail);
}
/* Checks whether string S matches each element of ACCEPTS. A list
@@ -1089,15 +1075,12 @@ in_acclist (const char *const *accepts, const char *s, bool backward)
char *
suffix (const char *str)
{
- int i;
+ char *p;
- for (i = strlen (str); i && str[i] != '/' && str[i] != '.'; i--)
-;
+ if ((p = strrchr(str, '.')) && !strchr(p + 1, '/'))
+ return p + 1;
- if (str[i++] == '.')
-return (char *)str + i;
- else
-return NULL;
+ return NULL;
}
/* Return true if S contains globbing wildcards (`*', `?', `[' or
@@ -1106,10 +1089,7 @@ suffix (const char *str)
bool
has_wildcards_p (const char *s)
{
- for (; *s; s++)
-
Re: [Bug-wget] [PATCH] Regression since wget 1.10: no_prefix function is *bad*
Greetings, Tim Rühsen. Possible that i'm understood wrong, but according to http://www.mingw.org/wiki/C99 MinGW doesn't support C99 at all. So i'm not sure about cross-compile. May be after implementation of C99 code it can be compiled only through CygWin (don't remember exactly, but two years ago Wget compiled at CygWin required on pure Windows only several libraries as dependencies. If need, i'll try to check this at week). P.S. Currently Wget compiled without any problem in Windows MinGW and resulting binary work in pure Windows without any dependencies. P.P.S. What "minor compile issue in url.c"? I get only error with undefined PC_NAME_MAX (just add it to compiler flags)? P.P.P.S. Sorry for bad English. Best regards, Alex. Thanks for the hint. I just installed Debians MinGW packages and it workedlike a charm (except a minor compile issue in url.c). For anyone who wants to try: $ make distclean $ ./configure --host=i686-w64-mingw32 --without-ssl --disable-ipv6 $ make $ wine src/wget.exe http://www.example.com I have no real Windows installation around to test wget.exe. But assuming, it works: Is there any need to stick at c89 code ? Or in other words: do we still have to support native Windows compilation withMSVC (couldn't these people install and use mingw) ? Regards, Tim
Re: [Bug-wget] [PATCH] Regression since wget 1.10: no_prefix function is *bad*
Am Sonntag, 12. Mai 2013 schrieb Ángel González: > On 12/05/13 21:50, Tim Rühsen wrote: > > A real solution would be a rewrite of the init stuff (I saw that already > > somewhere on the Wget 2.0 wish list or somewhere - don't remeber exactly). > > > > I already wrote this kind of code and would contribute it to Wget. > > But i am unshure how to apply it to Wget. Since it would be a pretty big > > change, should i git-clone Wget and you merge later or do you create a new > > branch or ... > > > > Ah, than we again have to discuss that infamous c89/c99 thing. > > AFAIR, the main argument against c99 came from Daniel Stenberg (Curl, haxx.se) > > who mentioned MS Visual C not being C99 ready (it will never be, said MS). > > I just saw that Debian has MinGW cross compiler packets for Win32 and Win64 > > with gcc 4.6, but I have no experience with those. > > Does anybody know if that is a real alternative to MS VC ? > > > > Regards, Tim > Yes, it is a real alternative as a compiler which works :) > However, I'm not sure how much does wget compile natively in win32 in > right now, > either with VC++ or gcc, mostly due to autoconf and gnulib detection. Thanks for the hint. I just installed Debians MinGW packages and it worked like a charm (except a minor compile issue in url.c). For anyone who wants to try: $ make distclean $ ./configure --host=i686-w64-mingw32 --without-ssl --disable-ipv6 $ make $ wine src/wget.exe http://www.example.com I have no real Windows installation around to test wget.exe. But assuming, it works: Is there any need to stick at c89 code ? Or in other words: do we still have to support native Windows compilation with MSVC (couldn't these people install and use mingw) ? Regards, Tim
Re: [Bug-wget] [PATCH] Regression since wget 1.10: no_prefix function is *bad*
On 12/05/13 21:50, Tim Rühsen wrote: > A real solution would be a rewrite of the init stuff (I saw that already > somewhere on the Wget 2.0 wish list or somewhere - don't remeber exactly). > > I already wrote this kind of code and would contribute it to Wget. > But i am unshure how to apply it to Wget. Since it would be a pretty big > change, should i git-clone Wget and you merge later or do you create a new > branch or ... > > Ah, than we again have to discuss that infamous c89/c99 thing. > AFAIR, the main argument against c99 came from Daniel Stenberg (Curl, > haxx.se) > who mentioned MS Visual C not being C99 ready (it will never be, said MS). > I just saw that Debian has MinGW cross compiler packets for Win32 and Win64 > with gcc 4.6, but I have no experience with those. > Does anybody know if that is a real alternative to MS VC ? > > Regards, Tim Yes, it is a real alternative as a compiler which works :) However, I'm not sure how much does wget compile natively in win32 in right now, either with VC++ or gcc, mostly due to autoconf and gnulib detection.
Re: [Bug-wget] [PATCH] Regression since wget 1.10: no_prefix function is *bad*
Am Sonntag, 12. Mai 2013 schrieb Giuseppe Scrivano: > Tim Rühsen writes: > > > having an abort() without a message is simply a big waste of time for any > > developer who stumbles upon it. > > I disagree here, what is so difficult that a debugger cannot catch? On > the other hand, I agree this can be improved. > > > > > Since the init code of Wget has to be rewritten anyways, i provide the fastest > > solution right now: increasing the buffer size and printing a message before > > Wget aborts. > > > > And yes, the whole issue is hell stupid... > > > - static char buffer[1024]; > > + static char buffer[2048]; > > > This won't really fix the problem of having a static buffer, the real > fix would be to dynamically allocate the memory. Yes, as I wrote, it is a quick hack. A real solution would be a rewrite of the init stuff (I saw that already somewhere on the Wget 2.0 wish list or somewhere - don't remeber exactly). I already wrote this kind of code and would contribute it to Wget. But i am unshure how to apply it to Wget. Since it would be a pretty big change, should i git-clone Wget and you merge later or do you create a new branch or ... Ah, than we again have to discuss that infamous c89/c99 thing. AFAIR, the main argument against c99 came from Daniel Stenberg (Curl, haxx.se) who mentioned MS Visual C not being C99 ready (it will never be, said MS). I just saw that Debian has MinGW cross compiler packets for Win32 and Win64 with gcc 4.6, but I have no experience with those. Does anybody know if that is a real alternative to MS VC ? Regards, Tim
Re: [Bug-wget] [PATCH] Regression since wget 1.10: no_prefix function is *bad*
Tim Rühsen writes: > having an abort() without a message is simply a big waste of time for any > developer who stumbles upon it. I disagree here, what is so difficult that a debugger cannot catch? On the other hand, I agree this can be improved. > Since the init code of Wget has to be rewritten anyways, i provide the > fastest > solution right now: increasing the buffer size and printing a message before > Wget aborts. > > And yes, the whole issue is hell stupid... > - static char buffer[1024]; > + static char buffer[2048]; This won't really fix the problem of having a static buffer, the real fix would be to dynamically allocate the memory. -- Giuseppe
[Bug-wget] [PATCH] Regression since wget 1.10: no_prefix function is *bad*
Hi Martin,
having an abort() without a message is simply a big waste of time for any
developer who stumbles upon it.
Since the init code of Wget has to be rewritten anyways, i provide the fastest
solution right now: increasing the buffer size and printing a message before
Wget aborts.
And yes, the whole issue is hell stupid...
Regards, Tim
Am Freitag, 10. Mai 2013 schrieb Marlin Frickenschmidt:
> Hello dear wget maintainers,
> I want to report a bug of sorts. It is not a direct bug that impedes the
> operation of wget to normal users, but one which basically makes it
> impossible to add more command-line options to wget without sooner or
> later making wget suddenly SIGABRT - without any inclination to show a
> proper error whatsover.
>
> The reason for this is the "no_prefix" function, which is supposed to
> prepend the string "no-" to a given string (for disabling certain
> command line options). The function makes the assumption that the total
> length of all command line option names together won't exceed 1024
> characters, because the buffer storing the strings for all the
> "no-"-prefixed command line options is only chosen that large. And so if
> it gets too big, there is just a silent abort(), no error message, no
> nothing.
>
> Why don't we use standard functions like asprintf to prepend strings,
> and instead build an own, completely broken function for it? Perhaps
> there are good reasons for building the string yourself instead of using
> the standard library; I don't know. In any case, there is no excuse for
> silently abort()ing without an error message. That is the only thing I
> am grumpy about, really...
>
> I hope this can be fixed in the next release for the sake of all wget
> developers around. I just spent two hours debugging this, and really
> couldn't believe my eyes when I found the reason for it. It is hell stupid.
>
> Cheers,
> Marlin
>
>
From e21315b9b4d41987479427ed203ae402695ec4df Mon Sep 17 00:00:00 2001
From: Tim Ruehsen
Date: Sat, 11 May 2013 17:11:51 +0200
Subject: [PATCH 3/3] warn before abort() in main.c/no_prefix
---
src/ChangeLog |4
src/main.c|7 +--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 3b6733e..bcff0f7 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,7 @@
+2013-05-11 Tim Ruehsen
+
+ * main.c (init_switches): warn before abort() in no_prefix()
+
2013-05-09 Tim Ruehsen
* cookies.c, ftp-ls.c, ftp.c, init.c, netrc.c, utils.c, utils.h:
diff --git a/src/main.c b/src/main.c
index 2b42d2d..70ce651 100644
--- a/src/main.c
+++ b/src/main.c
@@ -320,13 +320,15 @@ static struct cmdline_option option_data[] =
static char *
no_prefix (const char *s)
{
- static char buffer[1024];
+ static char buffer[2048];
static char *p = buffer;
char *cp = p;
int size = 3 + strlen (s) + 1; /* "no-STRING\0" */
- if (p + size >= buffer + sizeof (buffer))
+ if (p + size >= buffer + sizeof (buffer)) {
+fprintf (stderr, _("Internal error: size of 'buffer' in main.c/no-prefix() must be increased\n"));
abort ();
+ }
cp[0] = 'n', cp[1] = 'o', cp[2] = '-';
strcpy (cp + 3, s);
@@ -352,6 +354,7 @@ init_switches (void)
{
char *p = short_options;
size_t i, o = 0;
+
for (i = 0; i < countof (option_data); i++)
{
struct cmdline_option *opt = &option_data[i];
--
1.7.10.4
