Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test/actionscript/ActionScriptTest.c fails on OpenBSD 4.9 #3

Closed
ehy opened this issue Dec 6, 2011 · 12 comments
Closed

test/actionscript/ActionScriptTest.c fails on OpenBSD 4.9 #3

ehy opened this issue Dec 6, 2011 · 12 comments

Comments

@ehy
Copy link

ehy commented Dec 6, 2011

Apparently BSD and GNU make treat macros differently.
ActionScriptTest.c loops over test cases from a string that
originates in a Makefile macro; and expects ' ' (space)
separators. Using isspace() lets the test run as expected.
(This is likely the same for any *BSD.)

diff -u:
--- test/actionscript/ActionScriptTest.c-orig 2011-10-26 02:33:18.000000000 -0400
+++ test/actionscript/ActionScriptTest.c 2011-12-06 10:28:06.000000000 -0500
@@ -39,6 +39,7 @@
#include <sys/types.h>
#include <sys/stat.h>
#include <limits.h>
+#include <ctype.h>
#include <makeswf.h>

static SWFMovie
@@ -82,18 +83,26 @@
const char *from, *to, *end;
char *ptr;
int version;

  • size_t len = strlen(all_tests);

from = all_tests;

  • end = from+strlen(all_tests);
  • end = from+len;
    do
    {
  •   while (*from && *from == ' ') ++from;
    
  •   /\* EH: for BSD make, change test ' ' to isspace() */
    
  •   while (*from && isspace(*from)) ++from;
    if ( ! *from ) break;
    
  •   to=strchr(from, ' ');
    
  •   /\* strtok()? too much trouble */
    
  •   for (to = from; !isspace(*to); to++) {
    
  •       if (*to == '\0') {
    
  •           to = NULL;
    
  •           break;
    
  •       }
    
  •   }
    if ( ! to ) to = end;
    
  •   size_t len = to-from;
    
  •   len = to-from;
    if ( len+1 >= PATH_MAX )
    {
        fprintf(stderr,
    
@ehy
Copy link
Author

ehy commented Dec 6, 2011

[sigh] trying the patch again:

diff -u:

--- test/actionscript/ActionScriptTest.c-orig   2011-10-26 02:33:18.000000000 -0400
+++ test/actionscript/ActionScriptTest.c    2011-12-06 10:28:06.000000000 -0500
@@ -39,6 +39,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <limits.h>
+#include <ctype.h>
 #include <makeswf.h>

 static SWFMovie
@@ -82,18 +83,26 @@
    const char *from, *to, *end;
    char *ptr;
    int version;
+   size_t len = strlen(all_tests);

    from = all_tests;
-   end = from+strlen(all_tests);
+   end = from+len;
    do
    {
-       while (*from && *from == ' ') ++from;
+       /* EH: for BSD make, change test ' ' to isspace() */
+       while (*from && isspace(*from)) ++from;
        if ( ! *from ) break;

-       to=strchr(from, ' ');
+       /* strtok()? too much trouble */
+       for (to = from; !isspace(*to); to++) {
+           if (*to == '\0') {
+               to = NULL;
+               break;
+           }
+       }
        if ( ! to ) to = end;

-       size_t len = to-from;
+       len = to-from;
        if ( len+1 >= PATH_MAX )
        {
            fprintf(stderr,

@ehy
Copy link
Author

ehy commented Dec 6, 2011

one more try.

--- test/actionscript/ActionScriptTest.c-orig   2011-10-26 02:33:18.000000000 -0400
+++ test/actionscript/ActionScriptTest.c    2011-12-06 10:28:06.000000000 -0500
@@ -39,6 +39,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <limits.h>
+#include <ctype.h>
 #include <makeswf.h>

 static SWFMovie
@@ -82,18 +83,26 @@
    const char *from, *to, *end;
    char *ptr;
    int version;
+   size_t len = strlen(all_tests);

    from = all_tests;
-   end = from+strlen(all_tests);
+   end = from+len;
    do
    {
-       while (*from && *from == ' ') ++from;
+       /* EH: for BSD make, change test ' ' to isspace() */
+       while (*from && isspace(*from)) ++from;
        if ( ! *from ) break;

-       to=strchr(from, ' ');
+       /* strtok()? too much trouble */
+       for (to = from; !isspace(*to); to++) {
+           if (*to == '\0') {
+               to = NULL;
+               break;
+           }
+       }
        if ( ! to ) to = end;

-       size_t len = to-from;
+       len = to-from;
        if ( len+1 >= PATH_MAX )
        {
            fprintf(stderr,

@strk
Copy link
Member

strk commented Dec 6, 2011

What is it, if not a space? A tab ? A newline ? Could it be simplified by using strstr rather than strchr ?
Also, could this be fixed at the Makefile level instead ?

@ehy
Copy link
Author

ehy commented Dec 6, 2011

On 12/06/2011 03:40 PM, strk wrote:

What is it, if not a space? A tab ? A newline ? Could it be simplified by using strstr rather than strchr ?
Also, could this be fixed at the Makefile level instead ?

They're tabs. The macro is like this:

AS_TESTS =
Function.as:6
ASM0.as:6
ASM1.as:6
ASM_all.as:6
ASM_extend.as:6
ASM_push.as:6
ASM_targetPath.as:6 \

etc., and BSD make is converting the leading tabs to spaces, but not
those following the non-whitespace; those are remaining tabs.

I know the for(;;) is ugly, and I considered strtok() for a moment, but
it can't work on const strings (modifies its args), requires different
args on 1st and subsequent calls; no prettier than the for loop.

As for strstr()? Since it works on strings, wouldn't it be error
prone if e.g. a " \t" got transposed at some point to "\t "? What
string to search for? It seemed to me isspace() is most general, even
if the loop stings the eyes.

The makefile could be fixed, but still, it's less general, and
something to forget in the future.

  • Ed

@ehy
Copy link
Author

ehy commented Dec 6, 2011

(Github stripped tabs from the macro sample, but they were there.)

@strk
Copy link
Member

strk commented Dec 7, 2011

Sorry, I meant strspn/strcspn rather than strstr

@strk
Copy link
Member

strk commented Dec 7, 2011

Please try the "bsd" branch in the repository, it contains a version using strcspn

@ehy
Copy link
Author

ehy commented Dec 7, 2011

On 12/07/2011 02:40 AM, strk wrote:

Sorry, I meant strspn/strcspn rather than strstr

OK, nice! Much prettier.

  • Ed

@ehy
Copy link
Author

ehy commented Dec 7, 2011

On 12/07/2011 02:49 AM, strk wrote:

Please try the "bsd" branch in the repository, it contains a version using strcspn

Got it, it's built. In test/Font listswf is dumping core.

    #0  0x1c01507b in parseSWF_GLYPHENTRY (f=0x2f12b140,

gerec=0x8acaa070, glyphbits=0, advancebits=9) at parser.c:245
245 gerec->GlyphIndex[0] = 0; /* for glyphbits == 0 */
(gdb) bt
#0 0x1c01507b in parseSWF_GLYPHENTRY (f=0x2f12b140,
gerec=0x8acaa070, glyphbits=0, advancebits=9) at parser.c:245
#1 0x1c0153e8 in parseSWF_TEXTRECORD (f=0x2f12b140,
brec=0x7f46d1e0, glyphbits=0, advancebits=9, level=1) at parser.c:302
#2 0x1c01cc23 in parseSWF_DEFINETEXT (f=0x2f12b140, length=28)
at parser.c:2262
#3 0x1c00a99f in blockParse (f=0x2f12b140, length=28,
header=SWF_DEFINETEXT) at blocktypes.c:145
#4 0x1c008aa5 in readMovie (f=0x2f12b140) at main.c:265
#5 0x1c008e89 in main (argc=2, argv=0xcfbe93c4) at main.c:350
(gdb) p gerec->GlyphIndex[0]
Cannot access memory at address 0x8a70a020
(gdb) p *gerec
$1 = {GlyphIndex = 0x8a70a020, GlyphAdvance = 0x0}

  • Ed

@strk
Copy link
Member

strk commented Dec 7, 2011

On Wed, Dec 07, 2011 at 06:26:19AM -0800, ehy wrote:

On 12/07/2011 02:49 AM, strk wrote:

Please try the "bsd" branch in the repository, it contains a version using strcspn

Got it, it's built. In test/Font listswf is dumping core.

Could you please file another issue for that ?
This one is for ActionScriptTest.c failure.

--strk;

,------o-.
| __/ | Thank you for PostGIS-2.0 Topology !
| / 2.0 | http://www.pledgebank.com/postgistopology
`-o------'

@strk
Copy link
Member

strk commented Dec 7, 2011

The fix for ActionScriptTest was pushed as d2e12f8

@strk strk closed this as completed Dec 7, 2011
@ehy
Copy link
Author

ehy commented Dec 7, 2011

On 12/07/2011 09:27 AM, strk wrote:

On Wed, Dec 07, 2011 at 06:26:19AM -0800, ehy wrote:

On 12/07/2011 02:49 AM, strk wrote:

Please try the "bsd" branch in the repository, it contains a version using strcspn

Got it, it's built. In test/Font listswf is dumping core.

Could you please file another issue for that ?
This one is for ActionScriptTest.c failure.

Done.

  • Ed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants