From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id B025F39AE012 for ; Tue, 20 Jul 2021 22:19:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B025F39AE012 Received: from pps.filterd (m0127361.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 16KMEgsH187328; Tue, 20 Jul 2021 18:19:57 -0400 Received: from ppma04wdc.us.ibm.com (1a.90.2fa9.ip4.static.sl-reverse.com [169.47.144.26]) by mx0a-001b2d01.pphosted.com with ESMTP id 39x73vr32t-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 20 Jul 2021 18:19:56 -0400 Received: from pps.filterd (ppma04wdc.us.ibm.com [127.0.0.1]) by ppma04wdc.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 16KMDdaW025833; Tue, 20 Jul 2021 22:19:56 GMT Received: from b01cxnp23033.gho.pok.ibm.com (b01cxnp23033.gho.pok.ibm.com [9.57.198.28]) by ppma04wdc.us.ibm.com with ESMTP id 39upubj6u4-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 20 Jul 2021 22:19:56 +0000 Received: from b01ledav006.gho.pok.ibm.com (b01ledav006.gho.pok.ibm.com [9.57.199.111]) by b01cxnp23033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 16KMJtB423986442 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 20 Jul 2021 22:19:55 GMT Received: from b01ledav006.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id AC8B5AC05E; Tue, 20 Jul 2021 22:19:55 +0000 (GMT) Received: from b01ledav006.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 4AA9AAC05F; Tue, 20 Jul 2021 22:19:55 +0000 (GMT) Received: from Bills-MacBook-Pro.local (unknown [9.211.140.71]) by b01ledav006.gho.pok.ibm.com (Postfix) with ESMTP; Tue, 20 Jul 2021 22:19:55 +0000 (GMT) Reply-To: wschmidt@linux.ibm.com Subject: Re: [PATCH 10/55] rs6000: Main function with stubs for parsing and output To: Segher Boessenkool Cc: gcc-patches@gcc.gnu.org References: <20210719191524.GP1583@gate.crashing.org> From: Bill Schmidt Message-ID: <96c1f140-9600-86a6-276f-2ac8c3fbec38@linux.ibm.com> Date: Tue, 20 Jul 2021 17:19:54 -0500 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <20210719191524.GP1583@gate.crashing.org> Content-Language: en-GB X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: qBkA9SI_CKxstTAdGAUwWcTfbceHnLfr X-Proofpoint-GUID: qBkA9SI_CKxstTAdGAUwWcTfbceHnLfr X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391, 18.0.790 definitions=2021-07-20_14:2021-07-19, 2021-07-20 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 phishscore=0 mlxscore=0 lowpriorityscore=0 impostorscore=0 adultscore=0 clxscore=1015 priorityscore=1501 spamscore=0 suspectscore=0 bulkscore=0 malwarescore=0 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2104190000 definitions=main-2107200138 X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, HTML_MESSAGE, NICE_REPLY_A, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Jul 2021 22:19:59 -0000 Hi Segher, On 7/19/21 2:15 PM, Segher Boessenkool wrote: > Hi! > > On Thu, Jun 17, 2021 at 10:18:54AM -0500, Bill Schmidt wrote: >> * config/rs6000/rs6000-gen-builtins.c (rbtree.h): New #include. >> (num_bifs): New variable. >> (num_ovld_stanzas): Likewise. >> (num_ovlds): Likewise. >> (parse_codes): New enum. >> (bif_rbt): New variable. >> (ovld_rbt): Likewise. >> (fntype_rbt): Likewise. >> (bifo_rbt): Likewise. >> (parse_bif): New stub function. >> (create_bif_order): Likewise. >> (parse_ovld): Likewise. >> (write_header_file): Likewise. >> (write_init_file): Likewise. >> (write_defines_file): Likewise. >> (delete_output_files): New function. >> (main): Likewise. >> +/* Parse the built-in file. */ >> +static parse_codes >> +parse_bif (void) >> +{ >> + return PC_OK; >> +} > Baby steps :-) > >> +/* Write everything to the header file (rs6000-builtins.h). */ >> +static int >> +write_header_file (void) >> +{ >> + return 1; >> +} > What does the return value mean? Please document it in a comment. Same > for other functions (where the function name does not make it obvious > what the return value is). > >> +static void >> +delete_output_files (void) >> +{ >> + /* Depending on whence we're called, some of these may already be >> + closed. Don't check for errors. */ >> + fclose (header_file); >> + fclose (init_file); >> + fclose (defines_file); >> + >> + unlink (header_path); >> + unlink (init_path); >> + unlink (defines_path); >> +} > What are header_path etc.? It is a very good idea to make sure this is > never something terrible to call unlink on (including making sure the > delete_output_files function is *obviously* never called if creating the > files didn't succeed). See the main function.  All three files are guaranteed to have been opened for writing when this is called, but some of them may have already been closed.  So the fclose calls may fail to do anything, but the unlinks will always delete the output files. This is done to avoid leaving garbage lying around after a parsing failure. > >> +/* Main program to convert flat files into built-in initialization code. */ >> +int >> +main (int argc, const char **argv) >> +{ >> + if (argc != 6) >> + { >> + fprintf (stderr, >> + "Five arguments required: two input file and three output " >> + "files.\n"); > Two input file_s_ :-) (Or s/file //). > >> + pgm_path = argv[0]; > This isn't portable (depending on what you use it for -- argv[0] is not > necessarily a path at all). The only thing it's used for is as a documentation string in the output files, indicating the path to the program that built them. So long as argv[0] is a NULL-terminated string, which it had better be, this is harmless. ISO C11:  "If the value of|argc|is greater than zero, the string pointed to by|argv[0]|represents the program name;|argv[0][0]|shall be the null character if the program name is not available from the host environment." So I think we're good here. > >> + bif_file = fopen (bif_path, "r"); >> + if (!bif_file) >> + { >> + fprintf (stderr, "Cannot find input built-in file '%s'.\n", bif_path); >> + exit (1); >> + } > Say s/find/open/ in the error? > >> + fprintf (stderr, "Cannot find input overload file '%s'.\n", ovld_path); > (more) > > Okay with those trivialities, and the unlink stuff looked at. Thanks! I'll get this cleaned up and post what I commit.  Thanks! Bill > > > Segher