From patchwork Sun Oct 19 14:21:44 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ramsay Jones X-Patchwork-Id: 5101071 Return-Path: X-Original-To: patchwork-linux-sparse@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 52EF19F30B for ; Sun, 19 Oct 2014 14:21:58 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 55D03201C7 for ; Sun, 19 Oct 2014 14:21:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1C3CD201BC for ; Sun, 19 Oct 2014 14:21:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751938AbaJSOVz (ORCPT ); Sun, 19 Oct 2014 10:21:55 -0400 Received: from mdfmta004.mxout.tbr.inty.net ([91.221.168.45]:34452 "EHLO smtp.demon.co.uk" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751916AbaJSOVy (ORCPT ); Sun, 19 Oct 2014 10:21:54 -0400 Received: from mdfmta004.tbr.inty.net (unknown [127.0.0.1]) by mdfmta004.tbr.inty.net (Postfix) with ESMTP id 69EC9A0C089; Sun, 19 Oct 2014 15:21:25 +0100 (BST) Received: from mdfmta004.tbr.inty.net (unknown [127.0.0.1]) by mdfmta004.tbr.inty.net (Postfix) with ESMTP id 2354DA0C086; Sun, 19 Oct 2014 15:21:25 +0100 (BST) Received: from [10.0.2.15] (unknown [80.176.147.220]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mdfmta004.tbr.inty.net (Postfix) with ESMTP; Sun, 19 Oct 2014 15:21:24 +0100 (BST) Message-ID: <5443C8F8.2000003@ramsay1.demon.co.uk> Date: Sun, 19 Oct 2014 15:21:44 +0100 From: Ramsay Jones User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Christopher Li CC: Sparse Mailing-list , Thomas Graf Subject: Re: [PATCH 2/4] cgcc: avoid passing a sparse-only option to cc References: <54398BA5.6050005@ramsay1.demon.co.uk> <543EAFFA.6070503@ramsay1.demon.co.uk> In-Reply-To: X-MDF-HostID: 9 Sender: linux-sparse-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-sparse@vger.kernel.org X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Sorry for the late reply, it's been a busy few days. ;-) On 16/10/14 02:45, Christopher Li wrote: > Hi, > > I create a branch in the chrisl repo call "review-ramsay" for your new > follow up patches. > I plan to do incremental fix up if any on that branch. Then when we > both happy about it, > I will do rebase and smash the commit before push to master branch. Yes, I noticed the new branch - thanks! Did you have any thoughts regarding Josh Triplett's comments on the "compile-i386.c: don't ignore return value of write(2)" patch? > > > > On Thu, Oct 16, 2014 at 1:33 AM, Ramsay Jones > wrote: >> On 15/10/14 15:23, Christopher Li wrote: >>> On Sun, Oct 12, 2014 at 3:57 AM, Ramsay Jones >>> wrote: >>>> >>>> >>>> - system ($check) == 0 or die; >>>> + system ($check) == 0 or exit 1; >>> >>> Why not exit with the error code from sparse here? >>> Sparse might exit with other error code than "1" in the future. >> >> The meaning of the return value from system() is not portable. > > I see. Does it mean we should use some thing other than "system". > or maybe some thing like: > > $retcode = system($check); > if $retcode !=0 { exit $retcode; } No, no, you don't want to do that even on Linux (let alone more exotic platforms like Windows, MinGW and cygwin). $ vim cgcc $ git diff $ $ CHECK=./sparse ./cgcc hello.c hello.c:3:5: warning: symbol 'f' was not declared. Should it be static? code is: 0 $ echo $? 0 $ CHECK=./sparse ./cgcc -Wsparse-error hello.c hello.c:3:5: error: symbol 'f' was not declared. Should it be static? code is: 256 $ echo $? 0 $ Note that the 'return code' from ./sparse (1) is encoded in the 2nd byte of $code. i.e. you have to 'interpret' the return from system in a similar manner to a C program inspecting the status returned by the wait() system call. (see the exit_code sub from the last email, which included a right shift by 8 bits). Hmm, well, maybe! It is often difficult, on many platforms, to determine if system() will run the program directly using execvp or indirectly using a system shell (and which one). There are various rules about this issue which depends on the *form* of the call to system (is the argument a simple scalar or an array) and if it is a scalar variable does it contain any 'shell metacharacters'. (And even if it doesn't, it still may use a shell anyway!) $ CHECK=./junk ./cgcc -Wsparse-error hello.c sh: 1: ./junk: not found code is: 32512 $ echo $? 0 $ Note that 32512 == 0x7f00 which means the return code from the shell was 127 which, according to the bash manpage means the command execution failed due to 'command not found'. (This was actually an execution of the dash shell, but I couldn't find the information in the dash manpage, and it seems to follow bash, which in turn is following the original bourne shell). ['man bash', then /EXIT]. It simply isn't worth the trouble ... :-D [This all presupposes that we are happy to accept the change in behaviour introduced by commit 4d881187 in the first place; i.e. not calling gcc if 'sparse' returns a non-zero status. ;-) ] ATB, Ramsay Jones --- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/cgcc b/cgcc index 8e38174..364c77c 100755 --- a/cgcc +++ b/cgcc @@ -83,7 +83,9 @@ if ($do_check) { print "$check\n" if $verbose; if ($do_compile) { - system ($check) == 0 or exit 1; + my $code = system ($check); + print "code is: $code\n" ; + exit $code if $code != 0; } else { exec ($check); }