Re: New ARI regression Fri Oct 23 01:57:01 UTC 2009

[Pedro Alves <pedro@codesourcery.com>]

On Friday 23 October 2009 18:12:29, Michael Snyder wrote:
> 2009-10-23  Michael Snyder  <msnyder@vmware.com>
> 
>         * record.c (netorder64): Add gdbarch argument.  Use for byte order.
>         (netorder32): Ditto.
>         (netorder16): Ditto.
>         (record_restore): Get gdbarch and pass to netorder.
>         (cmd_record_save): Ditto.
> 
> Index: record.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/record.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 record.c
> --- record.c    23 Oct 2009 14:35:30 -0000      1.32
> +++ record.c    23 Oct 2009 17:17:21 -0000
> @@ -31,7 +31,6 @@
>  #include "elf-bfd.h"
>  #include "gcore.h"
>  
> -#include <byteswap.h>
>  #include <signal.h>
>  
>  /* This module implements "target record", also known as "process
> @@ -59,7 +58,7 @@
>  #define RECORD_IS_REPLAY \
>       (record_list->next || execution_direction == EXEC_REVERSE)
>  
> -#define RECORD_FILE_MAGIC      netorder32(0x20091016)
> +#define RECORD_FILE_MAGIC      netorder32(gdbarch, 0x20091016)

I'd still make the magic target order independent.  As simple as:

static unsigned char record_file_magic[] = { 0x20, 0x09, 0x10, 0x16 };

and then use straight memcmp to compare this:

	read 4 bytes of header into buf
        
        if (memcmp (buf, record_file_magic, 4) == 0)
          success!!

Or simply store_unsigned_integer/extract_unsigned_integer
with one of BFD_ENDIAN_LITTLE|BIG hardcoded...

I'd also add a simple version field after the magic; a simple
incrementing integer, so that you don't have to keep
inventing a new magic for every bump.

>  static inline uint64_t
> -netorder64 (uint64_t fromfile)
> +netorder64 (struct gdbarch *gdbarch, uint64_t input)
>  {
> -  return (BYTE_ORDER == LITTLE_ENDIAN) 
> -    ? bswap_64 (fromfile) 
> -    : fromfile;
> +  uint64_t ret;
> +
> +  store_unsigned_integer ((gdb_byte *) &ret, sizeof (ret), 
> +                         gdbarch_byte_order (gdbarch), input);
> +  return ret;
>  }

These functions aren't always "network order" now, and are
hence misnamed.  Rename or eliminate them by inlining they
bodies at the call sites.  Alternatively, if you wanted to
keep using "netorder", you could just simply always
hardcode BFD_ENDIAN_BIG, or pull htonl from gnulib.

-- 
Pedro Alves