"dir" crashing oactve 3.2.0/mingw32

[Benjamin Lindner]

Hello,

there have been some reports that simply calling "dir" crashes octave
3.2.0/mingw32 on some windows platforms.
This has been tracked down to calls to strftime() failing with a "%T"
format specifier.

Mingw uses the strftime function provided by microsoft C runtime
library, and indeed msdn states that the following format specifiers are
supported: aAbBcdHIjmMpSUwWxXyYzZ

Mind that "T" is not supported, neither is "e".

Don't ask me why MS does not simply ignore other format specifiers, but
causes applications to crash. But changing "%T" to the equivalent
"%H:%M:%S" fixes the crashes in "dir".
Since "e" is neither supported, I propose to change it to "d" (with "%e"
being sprintf("%d",dayofmonth) and "%d" being sprintf("%02d",dayofmonth).

See the attached changeset.

It would be great to have this also fixed in 3.2.x

benjamin

# HG changeset patch
# User Benjamin Lindner <lindnerb@...>
# Date 1246121737 -7200
# Node ID 17dd42c0f54da8460aab457c7a631aa25a4cf1a0
# Parent  0dc089723162791456f0ae1c63dbe7e6790def8c
strftime format specifier fixes for MSCRT

diff -r 0dc089723162 -r 17dd42c0f54d scripts/ChangeLog
--- a/scripts/ChangeLog Fri Jun 26 16:20:02 2009 +0200
+++ b/scripts/ChangeLog Sat Jun 27 18:55:37 2009 +0200
@@ -1,3 +1,8 @@
[phone]  Benjamin Lindner <lindnerb@...>
+
+ * image/iminfo.m, miscellaneous/dir.m: change unsupported strftime
+ format specifiers %T and %e on win32.
+
 2009-06-25  Ben Abbott <bpabbott@...>
 
  * plot/gnuplot_drawnow.m: Apply feature 'wxt_has_size'.
diff -r 0dc089723162 -r 17dd42c0f54d scripts/image/imfinfo.m
--- a/scripts/image/imfinfo.m Fri Jun 26 16:20:02 2009 +0200
+++ b/scripts/image/imfinfo.m Sat Jun 27 18:55:37 2009 +0200
@@ -124,7 +124,7 @@
       error ("imfinfo: error reading '%s': %s", fn, msg);
     endif
 
-    time_stamp = strftime ("%e-%b-%Y %H:%M:%S", localtime (statinfo.mtime));
+    time_stamp = strftime ("%d-%b-%Y %H:%M:%S", localtime (statinfo.mtime));
   
     info = __magick_finfo__ (fn);
     info.FileModDate = time_stamp;
diff -r 0dc089723162 -r 17dd42c0f54d scripts/miscellaneous/dir.m
--- a/scripts/miscellaneous/dir.m Fri Jun 26 16:20:02 2009 +0200
+++ b/scripts/miscellaneous/dir.m Sat Jun 27 18:55:37 2009 +0200
@@ -114,7 +114,7 @@
   fn = cstrcat (fn, ext);
   info(i,1).name = fn;
   lt = localtime (st.mtime);
-  info(i,1).date = strftime ("%d-%b-%Y %T", lt);
+  info(i,1).date = strftime ("%d-%b-%Y %H:%M:%S", lt);
   info(i,1).bytes = st.size;
   info(i,1).isdir = S_ISDIR (st.mode);
   info(i,1).datenum = datenum (lt.year + 1900, lt.mon, lt.mday,
diff -r 0dc089723162 -r 17dd42c0f54d src/ChangeLog
--- a/src/ChangeLog Fri Jun 26 16:20:02 2009 +0200
+++ b/src/ChangeLog Sat Jun 27 18:55:37 2009 +0200
@@ -1,3 +1,8 @@
[phone]  Benjamin Lindner <lindnerb@...>
+
+ * load-save.cc: change unsupported strftime format specifiers %T and %e
+ on win32
+
 2009-06-26  John W. Eaton  <jwe@...>
 
  * load-path.cc (Faddpath): Preserve order of prepended elements.
diff -r 0dc089723162 -r 17dd42c0f54d src/load-save.cc
--- a/src/load-save.cc Fri Jun 26 16:20:02 2009 +0200
+++ b/src/load-save.cc Sat Jun 27 18:55:37 2009 +0200
@@ -1183,7 +1183,7 @@
  memset (headertext, ' ', 124);
  // ISO 8601 format date
  strftime (headertext, 124, "MATLAB 5.0 MAT-file, written by Octave "
-  OCTAVE_VERSION ", %Y-%m-%d %T UTC", &bdt);
+  OCTAVE_VERSION ", %Y-%m-%d %H:%M:%S UTC", &bdt);
 
  // The first pair of bytes give the version of the MAT file
  // format.  The second pair of bytes form a magic number which

[Jaroslav Hajek-2]

On Sat, Jun 27, 2009 at 6:56 PM, Benjamin Lindner<lindnerben@...> wrote:

> Hello,
>
> there have been some reports that simply calling "dir" crashes octave
> 3.2.0/mingw32 on some windows platforms.
> This has been tracked down to calls to strftime() failing with a "%T" format
> specifier.
>
> Mingw uses the strftime function provided by microsoft C runtime library,
> and indeed msdn states that the following format specifiers are supported:
> aAbBcdHIjmMpSUwWxXyYzZ
>
> Mind that "T" is not supported, neither is "e".
>
> Don't ask me why MS does not simply ignore other format specifiers, but
> causes applications to crash. But changing "%T" to the equivalent "%H:%M:%S"
> fixes the crashes in "dir".
> Since "e" is neither supported, I propose to change it to "d" (with "%e"
> being sprintf("%d",dayofmonth) and "%d" being sprintf("%02d",dayofmonth).
>
> See the attached changeset.
>
> It would be great to have this also fixed in 3.2.x
>
> benjamin

I don't think this is a good solution, since using %e in strftime will
still crash. It would be much better if the replacements were
conditionally (on windows) done directly in strftime.

regards

--
RNDr. Jaroslav Hajek
computing expert & GNU Octave developer
Aeronautical Research and Test Institute (VZLU)
Prague, Czech Republic
url: www.highegg.matfyz.cz

[John W. Eaton-3]

On  2-Jul-2009, Jaroslav Hajek wrote:

| On Sat, Jun 27, 2009 at 6:56 PM, Benjamin Lindner<lindnerben@...> wrote:
| > Hello,
| >
| > there have been some reports that simply calling "dir" crashes octave
| > 3.2.0/mingw32 on some windows platforms.
| > This has been tracked down to calls to strftime() failing with a "%T" format
| > specifier.
| >
| > Mingw uses the strftime function provided by microsoft C runtime library,
| > and indeed msdn states that the following format specifiers are supported:
| > aAbBcdHIjmMpSUwWxXyYzZ
| >
| > Mind that "T" is not supported, neither is "e".
| >
| > Don't ask me why MS does not simply ignore other format specifiers, but
| > causes applications to crash. But changing "%T" to the equivalent "%H:%M:%S"
| > fixes the crashes in "dir".
| > Since "e" is neither supported, I propose to change it to "d" (with "%e"
| > being sprintf("%d",dayofmonth) and "%d" being sprintf("%02d",dayofmonth).
| >
| > See the attached changeset.
| >
| > It would be great to have this also fixed in 3.2.x
| >
| > benjamin
|
|
| I don't think this is a good solution, since using %e in strftime will
| still crash. It would be much better if the replacements were
| conditionally (on windows) done directly in strftime.

How about this change instead?

  http://hg.savannah.gnu.org/hgweb/octave/rev/69d05d1a63b9

jwe

[Jaroslav Hajek-2]

On Thu, Jul 9, 2009 at 9:05 PM, John W. Eaton<jwe@...> wrote:

> On  2-Jul-2009, Jaroslav Hajek wrote:
>
> | On Sat, Jun 27, 2009 at 6:56 PM, Benjamin Lindner<lindnerben@...> wrote:
> | > Hello,
> | >
> | > there have been some reports that simply calling "dir" crashes octave
> | > 3.2.0/mingw32 on some windows platforms.
> | > This has been tracked down to calls to strftime() failing with a "%T" format
> | > specifier.
> | >
> | > Mingw uses the strftime function provided by microsoft C runtime library,
> | > and indeed msdn states that the following format specifiers are supported:
> | > aAbBcdHIjmMpSUwWxXyYzZ
> | >
> | > Mind that "T" is not supported, neither is "e".
> | >
> | > Don't ask me why MS does not simply ignore other format specifiers, but
> | > causes applications to crash. But changing "%T" to the equivalent "%H:%M:%S"
> | > fixes the crashes in "dir".
> | > Since "e" is neither supported, I propose to change it to "d" (with "%e"
> | > being sprintf("%d",dayofmonth) and "%d" being sprintf("%02d",dayofmonth).
> | >
> | > See the attached changeset.
> | >
> | > It would be great to have this also fixed in 3.2.x
> | >
> | > benjamin
> |
> |
> | I don't think this is a good solution, since using %e in strftime will
> | still crash. It would be much better if the replacements were
> | conditionally (on windows) done directly in strftime.
>
> How about this change instead?
>
>  http://hg.savannah.gnu.org/hgweb/octave/rev/69d05d1a63b9
>
> jwe
>

I transplanted it to 3.2.x


--
RNDr. Jaroslav Hajek
computing expert & GNU Octave developer
Aeronautical Research and Test Institute (VZLU)
Prague, Czech Republic
url: www.highegg.matfyz.cz

[Benjamin Lindner]

Jaroslav Hajek wrote:

> On Thu, Jul 9, 2009 at 9:05 PM, John W. Eaton<jwe@...> wrote:
>> On  2-Jul-2009, Jaroslav Hajek wrote:
>>
>> | On Sat, Jun 27, 2009 at 6:56 PM, Benjamin Lindner<lindnerben@...> wrote:
>> | > Hello,
>> | >
>> | > there have been some reports that simply calling "dir" crashes octave
>> | > 3.2.0/mingw32 on some windows platforms.
>> | > This has been tracked down to calls to strftime() failing with a "%T" format
>> | > specifier.
>> | >
>> | > Mingw uses the strftime function provided by microsoft C runtime library,
>> | > and indeed msdn states that the following format specifiers are supported:
>> | > aAbBcdHIjmMpSUwWxXyYzZ
>> | >
>> | > Mind that "T" is not supported, neither is "e".
>> | >
>> | > Don't ask me why MS does not simply ignore other format specifiers, but
>> | > causes applications to crash. But changing "%T" to the equivalent "%H:%M:%S"
>> | > fixes the crashes in "dir".
>> | > Since "e" is neither supported, I propose to change it to "d" (with "%e"
>> | > being sprintf("%d",dayofmonth) and "%d" being sprintf("%02d",dayofmonth).
>> | >
>> | > See the attached changeset.
>> | >
>> | > It would be great to have this also fixed in 3.2.x
>> | >
>> | > benjamin
>> |
>> |
>> | I don't think this is a good solution, since using %e in strftime will
>> | still crash. It would be much better if the replacements were
>> | conditionally (on windows) done directly in strftime.
>>
>> How about this change instead?
>>
>>  http://hg.savannah.gnu.org/hgweb/octave/rev/69d05d1a63b9
>>
>> jwe
>>
>
> I transplanted it to 3.2.x
>

just back from holiday and catching up on unread emails.
thanks for the fix, it's of course the right thing to do.

benjamin

[Benjamin Lindner]

>
> just back from holiday and catching up on unread emails.
> thanks for the fix, it's of course the right thing to do.
>
> benjamin
>

Hmm, the patch introduces a regression when building both 3.2.x and
current development tip.
At linking stage of liboctave ld.exe fails with missing symbols related
to tzname.

I found that liboctave/strftime.c declares

#if HAVE_TZNAME
extern OCTAVE_IMPORT char *tzname[];
#endif

However, on mingw32, tzname[] is declared in <time.h> which is included
by liboctave/strftime.c. The specific declaration in <time.h> does not
match the one in strftime.c and I suppose the declaration in <time.h> is
hidden by the declaration in strftime.c.

There is a config.h preprocessor macro HAVE_DECL_TZNAME, which according
to config.h is defined if a tzname declaration is present. So the
double-declaration is redundant and in the case of mingw32 breaks linking.

My suggestion is to change strftime.c to

#if HAVE_TZNAME && !HAVE_DECL_TZNAME
extern OCTAVE_IMPORT char *tzname[];
#endif

as the attached changeset does.

benjamin


# HG changeset patch
# User Benjamin Lindner <lindnerb@...>
# Date 1250187632 -7200
# Node ID fba4a85c73af16413b28ae284c4d897f3f23d961
# Parent  8cf7b375ad6f0428fc99a8ed1b99f82db99b03a5
skip double declaration of tztime

diff -r 8cf7b375ad6f -r fba4a85c73af liboctave/strftime.c
--- a/liboctave/strftime.c Thu Aug 13 19:24:34 2009 +0200
+++ b/liboctave/strftime.c Thu Aug 13 20:20:32 2009 +0200
@@ -49,7 +49,7 @@
 # endif
 #endif
 
-#if HAVE_TZNAME
+#if HAVE_TZNAME && !HAVE_DECL_TZNAME
 extern OCTAVE_IMPORT char *tzname[];
 #endif