From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id CFF8C3858C53 for ; Wed, 10 Aug 2022 15:48:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CFF8C3858C53 Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-66-CsikJs30N7OG_hu_Rnl5Lg-1; Wed, 10 Aug 2022 11:48:31 -0400 X-MC-Unique: CsikJs30N7OG_hu_Rnl5Lg-1 Received: by mail-wm1-f69.google.com with SMTP id v64-20020a1cac43000000b003a4bea31b4dso1267158wme.3 for ; Wed, 10 Aug 2022 08:48:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:subject:from:references:to :content-language:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc; bh=Obwf2nQsdrT2acfu0qRrlDtIi3aQzMeLwCKkHJljWLQ=; b=7uBzas/HP/tDatfUI2iPoSuemekWvX0LJqxh2trg0mT7dZJv/DP0XABrV1E33FyLYW eXZ909GnC0w8oRmjmip4bI3ciDYtS9hAN8MBrvs9fsl3HzVRvY8Ra8tOso3W3StoUOIx 7mAkv5JljYdBHMEnB8M2eyqLA9kuOh1qTHlW11cRte22Yt6sx1Xhyp1wvcPmwetHaBzU HIIRHFz/1Pb2YT2bHsB8csqLkFRgCShZUgMlQAgNKoQY5LzJT2pKPhld2Z1VXt1DDD1n 3V5PqrX4Kj24IbJq57YOsvzRzRFlPrEvxjqbSZ74lVr24FXbuZ2CnV3RrHM/48mSK3AT kgCw== X-Gm-Message-State: ACgBeo3t7Hd0oMaBRYda3NrC8cWDpoOyEsztKW5VbaTQ54CbSyUohArt eYwTV7hN0jzZnzoRlVp6KTW+nSJ3I2/vmMdu7VDz/yQNrSv++tpIPQHdPSRd79f6eGJHgQoyMOa 0nIW3vUdUN5IBWqOmYA== X-Received: by 2002:a7b:cb47:0:b0:3a3:24f4:434 with SMTP id v7-20020a7bcb47000000b003a324f40434mr2886770wmj.195.1660146510004; Wed, 10 Aug 2022 08:48:30 -0700 (PDT) X-Google-Smtp-Source: AA6agR4v/5HasrNRsOVyyeSLBnfgAkiPBVHlfUcoGwOEbIzhzKcMgYTvD7a9MxGGZfKhFctK4dLLNA== X-Received: by 2002:a7b:cb47:0:b0:3a3:24f4:434 with SMTP id v7-20020a7bcb47000000b003a324f40434mr2886757wmj.195.1660146509776; Wed, 10 Aug 2022 08:48:29 -0700 (PDT) Received: from [192.168.1.6] (adsl-2-solo-236-177.claranet.co.uk. [80.168.236.177]) by smtp.gmail.com with ESMTPSA id l36-20020a05600c08a400b003a5260b8392sm3339705wmp.23.2022.08.10.08.48.29 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 10 Aug 2022 08:48:29 -0700 (PDT) Message-ID: <1b16772c-59bb-4b66-7ea7-29701c94a3db@redhat.com> Date: Wed, 10 Aug 2022 16:48:28 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 To: Andrew Burgess , binutils@sourceware.org References: From: Nick Clifton Subject: Re: [PATCH 2/3] objdump: introduce OBJDUMP_COLORS environment variable In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-GB Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Aug 2022 15:48:33 -0000 Hi Andrew, > Add OBJDUMP_COLORS environment variable, and allow this to control the > default behaviour of objdump when --disassembler-color is not used, > and we are disassembling to a terminal. Whilst this is a nice idea, there is a problem with using environment variables: users often fail to include them when reporting a bug. Imagine for example a bug report about the colored disassembly being broken somehow and the user has forgotten to mention that they have OBJDUMP_COLORS set to "extended". We could waste hours trying to track down the problem only to find that we had been looking at the wrong piece of code. I cannot think of a nice way to resolve this problem. One idea that does spring to mind is to have objdump generate a message to stdout when the environment variable is used. That way there is a good likelihood that the message will be included in a bug report. But this does mean new, extraneous output will now appear in the disassembly output. > +static enum color_selection > +objdump_default_disassembler_color_mode (void) > +{ > + enum color_selection mode; > + > + if (isatty (1)) > + { > + const char *tmp = getenv (objdump_colors_var); > + > + if (tmp == NULL || strncmp (tmp, "color", 5) == 0) Why strncmp() rather than strcmp () ? [Because of patch 3/3 I expect...] > + mode = on; > + if (strncmp (tmp, "extended", 8) == 0) > + mode = extended; > + else if (strncmp (tmp, "off", 3) == 0) > + mode = off; > + else > + mode = on; It would probably be best to check for strncmp (tmp, "on") here and report if there is no match - that way users will know that they have a malformed OBJDUMP_COLOR environment variable. Cheers Nick