From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x42f.google.com (mail-pf1-x42f.google.com [IPv6:2607:f8b0:4864:20::42f]) by sourceware.org (Postfix) with ESMTPS id 6FB1F385828C; Mon, 26 Sep 2022 13:30:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6FB1F385828C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pf1-x42f.google.com with SMTP id e68so6725723pfe.1; Mon, 26 Sep 2022 06:30:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date; bh=On/lpqJBsgGSovdPjktPflLaUykvzythO5akuuwkuao=; b=Ex/bfQyo/sMof7fB+s79MPK8/UMQ5iF6edRwJFHg5JssB/q0llnwv058g3QXntvEzE 2mu/wOglyjIKy/m/NU+iV4C0Ut2ul8G5qBeAfz0aqzj961PG+SygDi/xfbQv/30+R+fz zAQtsQQnnrnMuduvaC4RSPFmk9hgxtzKL/yWgG5GfXJ7oV3N6OMlpA6nBvOaDA0wmnqR Zz0tIQF3/EUZ8g1ZMyQbpq45qO9k796vD/ZogkJfJR1lYzgdE2/EcDrvs9reYrA3Dhk1 UOQKU33NKLr0PrVSrF79tDWUndMVONdJr/14j7boKmwq4Xf/+3Pr7Pcc+f/P9USShYeU GmIQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date; bh=On/lpqJBsgGSovdPjktPflLaUykvzythO5akuuwkuao=; b=MHjBN3QbQJdGybBPPbT0N6g4MbT301TIxdDdzLUR/5bTvdPuZYei55Q9gEWFSiDL4R RAgSum5tLg2KlYFXjEaiRgeTKObqlqvSvbFfeQpmeuAbJnw1A2e7T6agXLhLkk1n1aNX zngy3BnKXmAWP0hMYShMQ4eJuD2dR30coV3AEmoxGH9aUA1rcS4n3crFgDRfeTkB+SML bjD8RJzlhtJ+wlasrI8gChuM6gaECKX7+1/gdthZ7i0kGdVDAZWphnb6yNk2s0DKs+Fk piBlnFj5CWTA3HKVlsu+EXdKMNQTpZyV+D6hDsoJ4pBUZSZEXlwLlpJ5uxjcN4ktZP9A I+pQ== X-Gm-Message-State: ACrzQf1Avv96e5o4dyLJ4Sa11L2iF/vibbDP3e0XCQvc7VhJxKnZsZN0 LUdnZjZcxyzOcX9zPbVWZMh+UlKLxEU= X-Google-Smtp-Source: AMsMyM753DWk/mgpcB9eN6Z16EYbdYEWISuKnYT71pOI8FtK9Q3fvwT9tx3XM6QLdk+erKGRTaC3nQ== X-Received: by 2002:a63:1a4c:0:b0:43b:e648:a7a4 with SMTP id a12-20020a631a4c000000b0043be648a7a4mr20125131pgm.7.1664199019114; Mon, 26 Sep 2022 06:30:19 -0700 (PDT) Received: from squeak.grove.modra.org (158.106.96.58.static.exetel.com.au. [58.96.106.158]) by smtp.gmail.com with ESMTPSA id a15-20020a1709027e4f00b00172c7dee22fsm11142309pln.236.2022.09.26.06.30.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 26 Sep 2022 06:30:18 -0700 (PDT) Received: by squeak.grove.modra.org (Postfix, from userid 1000) id 6A70D114076E; Mon, 26 Sep 2022 23:00:15 +0930 (ACST) Date: Mon, 26 Sep 2022 23:00:15 +0930 From: Alan Modra To: Fangrui Song Cc: Simon Marchi , Jan Beulich , Nick Clifton , binutils@sourceware.org, gdb-patches@sourceware.org Subject: Re: [PATCH v3] binutils, gdb: support zstd compressed debug sections Message-ID: References: <20220923040837.550160-1-maskray@google.com> <7eadf897-9370-5a00-ae57-8e07251b8702@polymtl.ca> <20220926072027.o6g6f33hximpe7y3@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220926072027.o6g6f33hximpe7y3@google.com> X-Spam-Status: No, score=-3030.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, Sep 26, 2022 at 12:20:27AM -0700, Fangrui Song wrote: > On 2022-09-26, Alan Modra wrote: > > Better to avoid the need.. > > > > binutils/ > > * configure.ac (msgpack): Use "AS_IF" rather than "if". > > * configure: Regenerate. > > ld/ > > * configure.ac (jansson): Use "AS_IF" rather than "if". > > * configure: Regenerate. > > > > Thanks. I've changed the zstd patch to use AS_IF, too. It probably wasn't necessary inside AC_DEFUN, but won't hurt either. > BTW: I think the `!= xno` trick for ancient shells is not needed. `!= > no` is used many times in binutils-gdb. Quite possibly true nowadays. I'm not going to be the one that removes them all. :-) Your patch looks OK to me, apart from a few formatting nits. Here are two examples: > @@ -150,9 +163,10 @@ bfd_compress_section_contents (bfd *abfd, sec_ptr sec, > sec->size = orig_uncompressed_size; > if (decompress) > { > - if (!decompress_contents (uncompressed_buffer > - + orig_compression_header_size, > - zlib_size, buffer, buffer_size)) > + if (!decompress_contents ( > + sec->compress_status == DECOMPRESS_SECTION_ZSTD, > + uncompressed_buffer + orig_compression_header_size, > + zlib_size, buffer, buffer_size)) > { > bfd_set_error (bfd_error_bad_value); > bfd_release (abfd, buffer); One of the formatting guidelines in https://www.gnu.org/prep/standards/html_node/Formatting.html#Formatting talks about emacs indenting of code. The above looks reasonable with your indenting by hand, but auto-indent will turn it into if (!decompress_contents ( sec->compress_status == DECOMPRESS_SECTION_ZSTD, uncompressed_buffer + orig_compression_header_size, zlib_size, buffer, buffer_size)) lining up function args with the open parenthesis. Inserting a couple of temporary vars is the nicest solution to keeping line length manageable. bool is_z = sec->compress_status == DECOMPRESS_SECTION_ZSTD; bfd_byte *p = uncompressed_buffer + orig_compression_header_size; if (!decompress_contents (is_z, p, zlib_size, buffer, buffer_size)) > @@ -569,7 +604,8 @@ bfd_init_section_decompress_status (bfd *abfd, sec_ptr sec) > sec->compressed_size = sec->size; > sec->size = uncompressed_size; > bfd_set_section_alignment (sec, uncompressed_alignment_power); > - sec->compress_status = DECOMPRESS_SECTION_SIZED; > + sec->compress_status = ch_type == ELFCOMPRESS_ZSTD ? DECOMPRESS_SECTION_ZSTD > + : DECOMPRESS_SECTION_ZLIB; > > return true; > } The same emacs auto-indent will ruin the above. Write sec->compress_status = (ch_type == ELFCOMPRESS_ZSTD ? DECOMPRESS_SECTION_ZSTD : DECOMPRESS_SECTION_ZLIB); with unnecessary parentheses to guide indentation. OK to commit with those things fixed. -- Alan Modra Australia Development Lab, IBM