Re: [flac-dev] [PATCH] Change test scripts shell to bash, to avoid lack of arithmetic support in dash, which is sh on Ubuntu 10.04

[Erik de Castro Lopo]

Jaren Stangret wrote:

> Erik,
> 
> I was thinking of doing this:
> export MALLOC_PERTURB_=$(awk 'BEGIN { srand(); print int(rand() * 32767 %
> 255 + 1) }')
> 
> Or would you prefer using 'date'?

Your's is probably better and this is probably an improvement:

awk 'BEGIN { srand(); print int(rand() * 255 + 1) }'

Awk's rand seems to be seeded by the current epoch seconds, but in this
case that doesn't matter.

Erik
-- 
--
Erik de Castro Lopo
http://www.mega-nerd.com/
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] [PATCH] Change test scripts shell to bash, to avoid lack of arithmetic support in dash, which is sh on Ubuntu 10.04

[Jaren Stangret]

Which I think srand actually uses 'date' as it's seed in this case...


On Tue, Mar 19, 2013 at 10:29 PM, Jaren Stangret  wrote:

> Erik,
>
> I was thinking of doing this:
> export MALLOC_PERTURB_=$(awk 'BEGIN { srand(); print int(rand() * 32767 %
> 255 + 1) }')
>
> Or would you prefer using 'date'?
>
>
> On Tue, Mar 19, 2013 at 10:22 PM, Erik de Castro Lopo <
> mle...@mega-nerd.com> wrote:
>
>> Jesse Weinstein wrote:
>>
>> > The subject line mostly says it all, but for reference, having
>> #!/bin/sh causes the following error:
>> >
>> > arithmetic expression: expecting primary: " % 255 + 1"
>>
>> Arithmetic support is actually not the issue, rather its that dash/sh
>> doesn't support $RANDOM. The solution to this is to replace $RANDOM
>> with $(date +%N) which is obviouslsy not all that random, but which
>> is close enough to random for this usage.
>>
>> Since Jaren is in the process of making the scripts POSIX shell
>> compliant, I think swicthing to bash is a bad idea :-).
>>
>> I was going to do the $RANDOM to $(date +%N) conversion after Jaren's
>> POSIX patches were all applied.
>>
>> Cheers,
>> Erik
>> --
>> --
>> Erik de Castro Lopo
>> http://www.mega-nerd.com/
>> ___
>> flac-dev mailing list
>> flac-dev@xiph.org
>> http://lists.xiph.org/mailman/listinfo/flac-dev
>>
>
>
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] [PATCH] Change test scripts shell to bash, to avoid lack of arithmetic support in dash, which is sh on Ubuntu 10.04

[Jaren Stangret]

Erik,

I was thinking of doing this:
export MALLOC_PERTURB_=$(awk 'BEGIN { srand(); print int(rand() * 32767 %
255 + 1) }')

Or would you prefer using 'date'?


On Tue, Mar 19, 2013 at 10:22 PM, Erik de Castro Lopo
wrote:

> Jesse Weinstein wrote:
>
> > The subject line mostly says it all, but for reference, having #!/bin/sh
> causes the following error:
> >
> > arithmetic expression: expecting primary: " % 255 + 1"
>
> Arithmetic support is actually not the issue, rather its that dash/sh
> doesn't support $RANDOM. The solution to this is to replace $RANDOM
> with $(date +%N) which is obviouslsy not all that random, but which
> is close enough to random for this usage.
>
> Since Jaren is in the process of making the scripts POSIX shell
> compliant, I think swicthing to bash is a bad idea :-).
>
> I was going to do the $RANDOM to $(date +%N) conversion after Jaren's
> POSIX patches were all applied.
>
> Cheers,
> Erik
> --
> --
> Erik de Castro Lopo
> http://www.mega-nerd.com/
> ___
> flac-dev mailing list
> flac-dev@xiph.org
> http://lists.xiph.org/mailman/listinfo/flac-dev
>
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] [PATCH] Change test scripts shell to bash, to avoid lack of arithmetic support in dash, which is sh on Ubuntu 10.04

[Erik de Castro Lopo]

Jesse Weinstein wrote:

> The subject line mostly says it all, but for reference, having #!/bin/sh 
> causes the following error:
> 
> arithmetic expression: expecting primary: " % 255 + 1"

Arithmetic support is actually not the issue, rather its that dash/sh
doesn't support $RANDOM. The solution to this is to replace $RANDOM
with $(date +%N) which is obviouslsy not all that random, but which
is close enough to random for this usage.

Since Jaren is in the process of making the scripts POSIX shell 
compliant, I think swicthing to bash is a bad idea :-).

I was going to do the $RANDOM to $(date +%N) conversion after Jaren's
POSIX patches were all applied.

Cheers,
Erik
-- 
--
Erik de Castro Lopo
http://www.mega-nerd.com/
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

[flac-dev] Failing case00a test (was Re: flac 1.3.0pre2 pre-release)

[Jesse Weinstein]

On Ubuntu 10.04.4 LTS, with default options (i.e. fresh checkout of
HEAD : 93c12c784ad0337f665c647d5378ea4c67d63fe4 , autogen.sh
&& ./configure && make) I get the exact same error when running (cd test
&& ./test_flac.sh ).

I know Erik has already confirmed this, but I thought I might as well add my 
report, also.

On Mon, 2013-03-18 at 23:40 -0500, Jaren Stangret wrote:
> I'd also like to note that this is happening for me with current HEAD
> (619b43df36b725e11e12db768d558b1930a583b6).
> 
> 
> On Mon, Mar 18, 2013 at 11:25 PM, Jaren Stangret 
> wrote:
> I've been trying to get the test suite cleaned up and even
> with the original "untouched" test suite via flac 1.3.0pre2,
> I'm getting a test suite failure.
> 
> 
> flac 1.3.0pre2
> 
> 
> configure options:
> ./configure --prefix=/usr --build=i686-pc-linux-gnu
> --host=i686-pc-linux-gnu --mandir=/usr/share/man
> --infodir=/usr/share/info --datadir=/usr/share
> --sysconfdir=/etc --localstatedir=/var/lib
> --disable-dependency-tracking --enable-thorough-tests
> --enable-exhaustive-tests --disable-valgrind-testing
> --disable-static --disable-dependency-tracking --disable-debug
> --enable-sse --disable-3dnow --disable-altivec
> --disable-doxygen-docs --disable-xmms-plugin --enable-cpplibs
> --enable-ogg
> 
> 
> 
> make fullcheck:
> This test fails with 'test_flac.sh':
> 
> 
> Testing --keep-foreign-metadata...
> round-trip test (wacky1.wav) encode... NOTE:
> --keep-foreign-metadata is a new feature; make sure to test
> the output file before deleting the original.
> decode... NOTE: --keep-foreign-metadata is a new feature; make
> sure to test the output file before deleting the original.
> compare... OK
> round-trip test (wacky2.wav) encode... NOTE:
> --keep-foreign-metadata is a new feature; make sure to test
> the output file before deleting the original.
> decode... NOTE: --keep-foreign-metadata is a new feature; make
> sure to test the output file before deleting the original.
> compare... OK
> round-trip test (wacky1.w64) encode... NOTE:
> --keep-foreign-metadata is a new feature; make sure to test
> the output file before deleting the original.
> decode... NOTE: --keep-foreign-metadata is a new feature; make
> sure to test the output file before deleting the original.
> compare... OK
> round-trip test (wacky2.w64) encode... NOTE:
> --keep-foreign-metadata is a new feature; make sure to test
> the output file before deleting the original.
> decode... NOTE: --keep-foreign-metadata is a new feature; make
> sure to test the output file before deleting the original.
> compare... OK
> round-trip test (wacky1.rf64) encode... NOTE:
> --keep-foreign-metadata is a new feature; make sure to test
> the output file before deleting the original.
> decode... NOTE: --keep-foreign-metadata is a new feature; make
> sure to test the output file before deleting the original.
> compare... OK
> round-trip test (wacky2.rf64) encode... NOTE:
> --keep-foreign-metadata is a new feature; make sure to test
> the output file before deleting the original.
> decode... NOTE: --keep-foreign-metadata is a new feature; make
> sure to test the output file before deleting the original.
> compare... OK
> Testing the metadata-handling properties of flac-to-flac
> encoding...
> case00a... Files case00a-expect.meta and out.meta differ
> ERROR: metadata does not match expected case00a-expect.meta
> make: *** [fullcheck] Error 1
> 
> 
> Anyone else hitting this or is just my system?
>
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

[flac-dev] [PATCH] Change test scripts shell to bash, to avoid lack of arithmetic support in dash, which is sh on Ubuntu 10.04

[Jesse Weinstein]

The subject line mostly says it all, but for reference, having #!/bin/sh causes 
the following error:

arithmetic expression: expecting primary: " % 255 + 1"

---
 test/test_flac.sh|2 +-
 test/test_grabbag.sh |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/test_flac.sh b/test/test_flac.sh
index 10981c0..257c5ea 100755
--- a/test/test_flac.sh
+++ b/test/test_flac.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 
 #  FLAC - Free Lossless Audio Codec
 #  Copyright (C) 2001,2002,2003,2004,2005,2006,2007,2008,2009  Josh Coalson
diff --git a/test/test_grabbag.sh b/test/test_grabbag.sh
index 5d9f9d1..959bda5 100755
--- a/test/test_grabbag.sh
+++ b/test/test_grabbag.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 
 #  FLAC - Free Lossless Audio Codec
 #  Copyright (C) 2001,2002,2003,2004,2005,2006,2007,2008,2009  Josh Coalson
-- 
1.7.0.4


___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] Patch to add Unicode filename support for win32 flac

[JonY]

On 3/20/2013 00:35, Janne Hyvärinen wrote:
>>
>> As for calling __wgetmainargs, I have some concerns about the security
>> implications:
>> LoadLibrary("msvcrt.dll") <- Which msvcrt? Theoretical security exploit.
> 
> There is msvcrt.dll in the System32 dir in all supported Windows 
> systems. That is what the function targets, but of course LoadLibrary 
> searches from exe's dir first. I think security exploit concerns are 
> warrantless, if you can place malicious replacement c-runtime dll in the 
> exe's path you have already won.
> 

Yeah, which is why I said it was theoretical.

I've seen code that use __ImageBase to over the import tables to find
out which MSVCR* DLL is used and use GetModuleHandleA to avoid LoadLibrary.

>>
>> I think it is best to link it directly, please use the following
>> prototype and call it directly:
>>
>> =
>> #ifdef _DLL
>> #define CALL_DLLIMPORT __declspec(dllimport)
>> #else
>> #define CALL_DLLIMPORT
>> #endif
>> int __cdecl CALL_DLLIMPORT __wgetmainargs(int*, wchar_t***, wchar_t***,
>> int, int*);
>> =
>>
>> This should simplify the error handling logic and help against
>> LoadLibrary handle leaks, though the leak should not be an issue in
>> practice since it is only called once. The symbol should also be present
>> in MSVCR* DLLs.
> 
> This alone does nothing. It requires linking with an object file that 
> then deals with the function. If we link against msvcrt.lib the flac.exe 
> binary will no longer be static and it won't work without external 
> runtimes (which would also be loaded from the exe's dir if they exist 
> there). Linking with msvcmrt.lib won't find the function and unicode 
> version msvcurt.lib causes this error:
> Error1error LNK2005: ___iob_func already defined in 
> msvcurt.lib(MSVCR110.dll)G:\test\LIBCMT.lib(_file.obj)test
> Error2error LNK1169: one or more multiply defined symbols 
> foundG:\test\Release\test.exetest
> 

There is no __wgetmainargs in the static libcmt? Interesting.




signature.asc
Description: OpenPGP digital signature
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] Patch to add Unicode filename support for win32 flac

[LRN]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 19.03.2013 20:35, Janne Hyvärinen wrote:
> 
> On 19.3.2013 15:49, JonY wrote:
>> On 3/19/2013 19:59, Janne Hyvärinen wrote:
>>> On 18.3.2013 12:25, Erik de Castro Lopo wrote:
 JonY wrote:
 
> Before anyone does anything, see __wgetmainargs 
> .
> 
> It can expand wildcards. Since it already provides
> argc/argv/env, it is more a less a drop-in replacement for
> the main() arguments.
 +1
 
 Erik
>>> Alright, here's a patch utilizing this function. There's a lot
>>> of changes here. Project files have a new configuration called
>>> "Release (UTF8)", intended to be used when building the command
>>> line tools. This project has the required UTF-8 define in place
>>> so all libraries expect things in encoded format. Regular Debug
>>> and Release configurations do not use any new tricks so 
>>> existing projects won't break when compiled with those
>>> settings.
>>> 
>>> I'm at work and couldn't do extensive testing, but command line
>>> FLAC.exe seems to perform everything right with this.
>>> 
>>> Metaflac probably requires some minor tweaks but I wanted to
>>> show some progress so that 1.3 doesn't slip out the door while
>>> I'm busy.
>>> 
>> 
>> As for calling __wgetmainargs, I have some concerns about the
>> security implications: LoadLibrary("msvcrt.dll") <- Which msvcrt?
>> Theoretical security exploit.
> 
> There is msvcrt.dll in the System32 dir in all supported Windows 
> systems. That is what the function targets, but of course
> LoadLibrary searches from exe's dir first. I think security exploit
> concerns are warrantless, if you can place malicious replacement
> c-runtime dll in the exe's path you have already won.
See [1] for the info. According to that article,
LoadLibraryA("msvcrt.dll") should be perfectly safe.


[1]
http://msdn.microsoft.com/en-us/library/windows/desktop/ms682586%28v=vs.85%29.aspx

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJRSJzFAAoJEOs4Jb6SI2CwmdMIAJZyqiZ+PjwivvZ0QM5l/DdD
aoS6zf5/crr6Dm6IMm58Sf/4RMIRUM4d/4wXZ4xKyQvP7wmxzzAN5QDTwHRXsDY7
CyjpPKv1NGiwOYux9fNUEQyo7eNvtl8BEg2pq7fXLH2h/kBnCtB7/V+X8OZqiSFD
na2YnUOpNxBq0LnmkM3gQqlXE9ajsBDUNYfFDHBVtu3ZXDUVwKhdH8kX2pTJcjeS
QSCwdxGS2uGVUj+E/+hUny3wcBEVN2z8gGxWzrGAMTcV4dYHlcD7cKXfG59eAw93
HomBZvrRLVss4aXrV2fGZ5VQm2AjlNRFKTNhtxkZ7npB4ManN0C0vuPxYMe3Uuk=
=6WoQ
-END PGP SIGNATURE-
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] Patch to add Unicode filename support for win32 flac

[Janne Hyvärinen]

On 19.3.2013 15:49, JonY wrote:
> On 3/19/2013 19:59, Janne Hyvärinen wrote:
>> On 18.3.2013 12:25, Erik de Castro Lopo wrote:
>>> JonY wrote:
>>>
 Before anyone does anything, see __wgetmainargs
 .

 It can expand wildcards. Since it already provides argc/argv/env, it is
 more a less a drop-in replacement for the main() arguments.
>>> +1
>>>
>>> Erik
>> Alright, here's a patch utilizing this function. There's a lot of
>> changes here.
>> Project files have a new configuration called "Release (UTF8)", intended
>> to be used when building the command line tools. This project has the
>> required UTF-8 define in place so all libraries expect things in encoded
>> format.
>> Regular Debug and Release configurations do not use any new tricks so
>> existing projects won't break when compiled with those settings.
>>
>> I'm at work and couldn't do extensive testing, but command line FLAC.exe
>> seems to perform everything right with this.
>>
>> Metaflac probably requires some minor tweaks but I wanted to show some
>> progress so that 1.3 doesn't slip out the door while I'm busy.
>>
> Should the following
> #if defined _MSC_VER || defined __MINGW32__
>
> be simplified to
> #ifdef WIN32
>
> ? Alternatively _WIN32. Since it's win32 and not something compiler
> specific.

Indeed. Not sure what I was thinking.

>
> As for the macros:
> +#define flac_vfprintf vfprintf_utf8
> +#define flac_fopen fopen_utf8
> +#define flac_stat _stat64_utf8
> ...
>
> I leave this up for Erik to decide, though I would have preferred not
> using rename macros at all.

I think this is the sanest way. Only few #ifdefs instead of wrapper 
functions filled with them that do essentially nothing on *nix.

>
> Not sure if these were intended:
> +#include  /* for flac_stat() */
> +#include  /* for flac_utime() */
> +#include  /* for flac_chmod() */

Nope. Bug from mass replace.

>
> As for calling __wgetmainargs, I have some concerns about the security
> implications:
> LoadLibrary("msvcrt.dll") <- Which msvcrt? Theoretical security exploit.

There is msvcrt.dll in the System32 dir in all supported Windows 
systems. That is what the function targets, but of course LoadLibrary 
searches from exe's dir first. I think security exploit concerns are 
warrantless, if you can place malicious replacement c-runtime dll in the 
exe's path you have already won.

>
> I think it is best to link it directly, please use the following
> prototype and call it directly:
>
> =
> #ifdef _DLL
> #define CALL_DLLIMPORT __declspec(dllimport)
> #else
> #define CALL_DLLIMPORT
> #endif
> int __cdecl CALL_DLLIMPORT __wgetmainargs(int*, wchar_t***, wchar_t***,
> int, int*);
> =
>
> This should simplify the error handling logic and help against
> LoadLibrary handle leaks, though the leak should not be an issue in
> practice since it is only called once. The symbol should also be present
> in MSVCR* DLLs.

This alone does nothing. It requires linking with an object file that 
then deals with the function. If we link against msvcrt.lib the flac.exe 
binary will no longer be static and it won't work without external 
runtimes (which would also be loaded from the exe's dir if they exist 
there). Linking with msvcmrt.lib won't find the function and unicode 
version msvcurt.lib causes this error:
Error1error LNK2005: ___iob_func already defined in 
msvcurt.lib(MSVCR110.dll)G:\test\LIBCMT.lib(_file.obj)test
Error2error LNK1169: one or more multiply defined symbols 
foundG:\test\Release\test.exetest

>
> In stat_utf8, dealing with 32bit/64bit time_t? The following check
> should really compile time rather than runtime:
> sizeof(*buffer) == sizeof(st)

Entire stat_utf8 function can be removed. The code was changed later to 
always use stat64 one. Though I'd assume compiler to optimize sizeof 
checks appropriately.

___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] Patch to add Unicode filename support for win32 flac

[JonY]

On 3/19/2013 19:59, Janne Hyvärinen wrote:
> On 18.3.2013 12:25, Erik de Castro Lopo wrote:
>> JonY wrote:
>>
>>> Before anyone does anything, see __wgetmainargs
>>> .
>>>
>>> It can expand wildcards. Since it already provides argc/argv/env, it is
>>> more a less a drop-in replacement for the main() arguments.
>> +1
>>
>> Erik
> 
> Alright, here's a patch utilizing this function. There's a lot of
> changes here.
> Project files have a new configuration called "Release (UTF8)", intended
> to be used when building the command line tools. This project has the
> required UTF-8 define in place so all libraries expect things in encoded
> format.
> Regular Debug and Release configurations do not use any new tricks so
> existing projects won't break when compiled with those settings.
> 
> I'm at work and couldn't do extensive testing, but command line FLAC.exe
> seems to perform everything right with this.
> 
> Metaflac probably requires some minor tweaks but I wanted to show some
> progress so that 1.3 doesn't slip out the door while I'm busy.
> 

Should the following
#if defined _MSC_VER || defined __MINGW32__

be simplified to
#ifdef WIN32

? Alternatively _WIN32. Since it's win32 and not something compiler
specific.

As for the macros:
+#define flac_vfprintf vfprintf_utf8
+#define flac_fopen fopen_utf8
+#define flac_stat _stat64_utf8
...

I leave this up for Erik to decide, though I would have preferred not
using rename macros at all.

Not sure if these were intended:
+#include  /* for flac_stat() */
+#include  /* for flac_utime() */
+#include  /* for flac_chmod() */

As for calling __wgetmainargs, I have some concerns about the security
implications:
LoadLibrary("msvcrt.dll") <- Which msvcrt? Theoretical security exploit.

I think it is best to link it directly, please use the following
prototype and call it directly:

=
#ifdef _DLL
#define CALL_DLLIMPORT __declspec(dllimport)
#else
#define CALL_DLLIMPORT
#endif
int __cdecl CALL_DLLIMPORT __wgetmainargs(int*, wchar_t***, wchar_t***,
int, int*);
=

This should simplify the error handling logic and help against
LoadLibrary handle leaks, though the leak should not be an issue in
practice since it is only called once. The symbol should also be present
in MSVCR* DLLs.

In stat_utf8, dealing with 32bit/64bit time_t? The following check
should really compile time rather than runtime:
sizeof(*buffer) == sizeof(st)

That's all for now, I have not actually tested with mingw yet.




signature.asc
Description: OpenPGP digital signature
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] flac 1.3.0pre2 pre-release

[Erik de Castro Lopo]

Jaren Stangret wrote:

> Testing the metadata-handling properties of flac-to-flac encoding...
> case00a... Files case00a-expect.meta and out.meta differ
> ERROR: metadata does not match expected case00a-expect.meta
> make: *** [fullcheck] Error 1
> 
> Anyone else hitting this or is just my system?

Yes, I'm getting this too.

On the list of things to fix before the release :-).

Erik
-- 
--
Erik de Castro Lopo
http://www.mega-nerd.com/
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev