Re: [Qemu-devel] [RISU PATCH v3 05/10] risu: add support compressed tracefiles

[Peter Maydell]

On 9 December 2016 at 11:48, Alex Bennée <address@hidden> wrote:
> This uses the magic of zlib's gzread/write interface to wrap the
> tracefile in compression. The code changes are tiny. I spent more time
> messing about with the configure/linker stuff to auto-detect bits.
>
> Signed-off-by: Alex Bennée <address@hidden>
> ---
>  Makefile  |  3 ++-
>  configure | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  risu.c    | 15 ++++++++++-----
>  3 files changed, 67 insertions(+), 6 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 4202c35..4a2ef02 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -14,6 +14,7 @@
>  include Makefile.in
>
>  CFLAGS ?= -g -Wall
> +LDFLAGS += -lz
>
>  PROG=risu
>  SRCS=risu.c comms.c risu_$(ARCH).c risu_reginfo_$(ARCH).c
> @@ -31,7 +32,7 @@ all: $(PROG) $(BINS)
>  dump: $(RISU_ASMS)
>
>  $(PROG): $(OBJS)
> -       $(CC) $(STATIC) $(CFLAGS) -o $@ $^
> +       $(CC) $(STATIC) $(CFLAGS) -o $@ $^ $(LDFLAGS)

(I think there's a stray space at the end of this line.)

>
>  %.risu.asm: %.risu.bin
>         ${OBJDUMP} -b binary -m $(ARCH) -D $^ > $@
> diff --git a/configure b/configure
> index f81bdb5..d11680f 100755
> --- a/configure
> +++ b/configure
> @@ -6,6 +6,10 @@ compile() {
>      $CC $CFLAGS -c -o ${1}.o ${1}.c 2>/dev/null
>  }
>
> +link() {
> +    $LD $LDFLAGS -l${2} -o ${1} ${1}.o 2>/dev/null
> +}

I would ask you to get the shell quoting right,
but I see we don't do that in the existing configure
code, so never mind.

> +
>  check_define() {
>      c=${tmp_dir}/check_define_${1}
>      cat > ${c}.c <<EOF
> @@ -34,6 +38,50 @@ guess_arch() {
>      fi
>  }
>
> +check_type() {
> +    c=${tmp_dir}/check_type_${1}
> +    cat > ${c}.c <<EOF
> +#include <inttypes.h>
> +#include <stdint.h>
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +
> +int main(void) { $1 thisone; return 0; }
> +EOF
> +    compile $c
> +}
> +
> +check_lib() {
> +    c=${tmp_dir}/check_lib${1}
> +    cat > ${c}.c <<EOF
> +#include <stdint.h>
> +#include <$2.h>
> +
> +int main(void) { $3; return 0; }
> +EOF
> +    compile $c && link $c $1
> +}
> +
> +generate_config() {
> +    cfg=config.h
> +    echo "generating config.h..."
> +
> +    echo "/* config.h - generated by the 'configure' script */" > $cfg
> +    echo "#ifndef CONFIG_H" >> $cfg
> +    echo "#define CONFIG_H 1" >> $cfg
> +
> +    if check_type uintptr_t ; then
> +        echo "#define HAVE_UINTPTR_T 1" >> $cfg
> +    fi
> +    if check_type socklen_t ; then
> +        echo "#define HAVE_SOCKLEN_T 1" >> $cfg
> +    fi

In what circumstances would we not have uintptr_t and
socklen_t types? We know we're building for Linux, after all.

> +
> +    echo "#endif /* CONFIG_H */" >> $cfg
> +
> +    echo "...done"
> +}
> +
>  generate_makefilein() {
>      m=Makefile.in
>      echo "generating Makefile.in..."
> @@ -93,6 +141,7 @@ done
>
>  CC="${CC-${CROSS_PREFIX}gcc}"
>  AS="${AS-${CROSS_PREFIX}as}"
> +LD="${LD-${CROSS_PREFIX}ld}"
>  OBJCOPY="${OBJCOPY-${CROSS_PREFIX}objcopy}"
>  OBJDUMP="${OBJDUMP-${CROSS_PREFIX}objdump}"
>
> @@ -100,6 +149,12 @@ if test "x${ARCH}" = "x"; then
>      guess_arch
>  fi
>
> +if ! check_lib z zlib "zlibVersion()"; then
> +    echo "Cannot find libz compression library"
> +    exit 1
> +fi
> +
> +generate_config
>  generate_makefilein
>
>  echo "type 'make' to start the build"
> diff --git a/risu.c b/risu.c
> index 173dd3c..3670bb1 100644
> --- a/risu.c
> +++ b/risu.c
> @@ -25,6 +25,7 @@
>  #include <sys/mman.h>
>  #include <fcntl.h>
>  #include <string.h>
> +#include <zlib.h>
>
>  #include "risu.h"
>
> @@ -32,6 +33,7 @@ void *memblock = 0;
>
>  int apprentice_socket, master_socket;
>  int trace_file = 0;
> +gzFile gz_trace_file;
>
>  sigjmp_buf jmpbuf;
>
> @@ -59,7 +61,7 @@ int read_sock(void *ptr, size_t bytes)
>
>  int write_trace(void *ptr, size_t bytes)
>  {
> -   size_t res = write(trace_file, ptr, bytes);
> +   size_t res = gzwrite(gz_trace_file, ptr, bytes);
>     return (res == bytes) ? 0 : 1;
>  }
>
> @@ -77,7 +79,7 @@ int write_sock(void *ptr, size_t bytes)
>
>  int read_trace(void *ptr, size_t bytes)
>  {
> -   size_t res = read(trace_file, ptr, bytes);
> +   size_t res = gzread(gz_trace_file, ptr, bytes);
>     return (res == bytes) ? 0 : 1;
>  }
>
> @@ -203,9 +205,10 @@ int master(int sock)
>  {
>     if (sigsetjmp(jmpbuf, 1))
>     {
> -      if (trace_file) {
> -         close(trace_file);
> -         fprintf(stderr,"\nDone...\n");
> +      if (trace_file)
> +      {
> +         gzclose(gz_trace_file);
> +         fprintf(stderr,"Done...\n");
>           return 0;
>        } else {
>           return report_match_status();
> @@ -321,6 +324,7 @@ int main(int argc, char **argv)
>       if (trace_fn)
>         {
>           trace_file = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
> +         gz_trace_file = gzdopen(trace_file, "wb9");
>         } else {
>           fprintf(stderr, "master port %d\n", port);
>           sock = master_connect(port);
> @@ -332,6 +336,7 @@ int main(int argc, char **argv)
>       if (trace_fn)
>         {
>           trace_file = open(trace_fn, O_RDONLY);
> +         gz_trace_file = gzdopen(trace_file, "rb");
>         } else {
>           fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
>           sock = apprentice_connect(hostname, port);
> --
> 2.11.0

Changes to the C code are fine.

thanks
-- PMM