#69 xwd format broken on x86_64

closed-fixed
encoding (15)
5
2007-03-16
2007-03-14
No

1.1.5rc1

running `xvidcap --format xwd` creates a bunch of text-dddd.xwd files, but none of them can be opened by xwud or ImageMagick. Both tools fail when trying to read the colormap.

Tracked this one down to xtoxwd.c:

line 65:

static void
swap_4byte (unsigned long *i)

should be:

static void
swap_4byte (uint32_t *i)

and line 215:

swap_n_4byte ((unsigned char *) &head,
sizeof (head) / sizeof (long));

should be:
swap_n_4byte ((unsigned char *) &head,
sizeof (head) / sizeof (uint32_t));

The first gives an incompatible-pointer warning. The second will result in not enough of the structure being byte-swapped since sizeof(long) == 8 on 64-bit systems (not 4).

Note: using uint32_t needs #include<stdint.h> (if present) or #include<intypes.h> (if present) or any other compile-time size hack.

Note: this code is really bad and should probably be rewritten to use ntohl() and ntohs() etc.

Discussion

  • Karl H. Beckers

    Karl H. Beckers - 2007-03-14
    • assigned_to: nobody --> charly4711
    • status: open --> pending-fixed
     
  • Karl H. Beckers

    Karl H. Beckers - 2007-03-14

    Logged In: YES
    user_id=782084
    Originator: NO

    fixed in svn
    Don't see how switching to ntohl could help the issue though, since it also uses "long" rather than the more specific uint32_t.
    Incidentally, could you check what types XGetPixel/XPutPixel work with on AMD64 ... it that unsigned long, too?

     
  • Darren Frith

    Darren Frith - 2007-03-14
    • status: pending-fixed --> open-fixed
     
  • Darren Frith

    Darren Frith - 2007-03-14

    Logged In: YES
    user_id=746032
    Originator: YES

    Yes, XGetPixel and XPutPixel use unsigned long:
    unsigned long XGetPixel(XImage *ximage, int x, int y);
    int XPutPixel(XImage *ximage, int x, int y, unsigned long pixel);

    The prototypes for ntohl() are as follows on Fedora Core 6 x86_64:

    #include <arpa/inet.h>
    uint32_t htonl(uint32_t hostlong);
    uint16_t htons(uint16_t hostshort);
    uint32_t ntohl(uint32_t netlong);
    uint16_t ntohs(uint16_t netshort);

    These deviate from the tradition, but for good reason.

    These functions are also optimised in Linux. For instance on i486 and above these are macros for a single line of assembler that uses the "bswap" machine opcode.

     
  • Karl H. Beckers

    Karl H. Beckers - 2007-03-16
    • status: open-fixed --> closed-fixed
     
  • Karl H. Beckers

    Karl H. Beckers - 2007-03-16

    Logged In: YES
    user_id=782084
    Originator: NO

    good, using htons and htonl in SVN, now.

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks