From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4553CC433E0 for ; Tue, 23 Jun 2020 11:59:15 +0000 (UTC) Received: from dpdk.org (dpdk.org [92.243.14.124]) by mail.kernel.org (Postfix) with ESMTP id D6A4C20771 for ; Tue, 23 Jun 2020 11:59:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D6A4C20771 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=dev-bounces@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9D3A41D629; Tue, 23 Jun 2020 13:59:13 +0200 (CEST) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 634BE1D5F7 for ; Tue, 23 Jun 2020 13:59:10 +0200 (CEST) IronPort-SDR: JXy20JwZnRKsihGXlmMv4eESswBp8VYeqqRYJtXDOyEKV76ErFOe/69aL9uf0U+RY4XhfTSPrg ZERY0QEtr5Jw== X-IronPort-AV: E=McAfee;i="6000,8403,9660"; a="143111801" X-IronPort-AV: E=Sophos;i="5.75,271,1589266800"; d="scan'208";a="143111801" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Jun 2020 04:59:10 -0700 IronPort-SDR: p59bNfv/8LDVKahsaoFEuLT1uYzlaHauFEdVJBM8ZtctLT/KZ1AdXJ8LdVGkPUSys5wx5HsAti Mk8waQ+uIwyQ== X-IronPort-AV: E=Sophos;i="5.75,271,1589266800"; d="scan'208";a="422977981" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.31.167]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 23 Jun 2020 04:59:08 -0700 Date: Tue, 23 Jun 2020 12:59:05 +0100 From: Bruce Richardson To: Neil Horman Cc: Dmitry Kozlyuk , dev@dpdk.org, Thomas Monjalon , robin.jarry@6wind.com Message-ID: <20200623115905.GA925@bricha3-MOBL.ger.corp.intel.com> References: <20200622004503.29036-1-dmitry.kozliuk@gmail.com> <20200622124117.GA216823@hmswarspite.think-freely.org> <20200622223946.25977c13@sovereign> <20200623112806.GA301645@hmswarspite.think-freely.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200623112806.GA301645@hmswarspite.think-freely.org> Subject: Re: [dpdk-dev] [RFC PATCH 0/2] pmdinfogen: rewrite in Python X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, Jun 23, 2020 at 07:28:06AM -0400, Neil Horman wrote: > On Mon, Jun 22, 2020 at 10:39:46PM +0300, Dmitry Kozlyuk wrote: > > On Mon, 22 Jun 2020 08:41:17 -0400, Neil Horman wrote: > > > On Mon, Jun 22, 2020 at 03:45:01AM +0300, Dmitry Kozlyuk wrote: > > [snip] > > > > > > 2. How much error-handling is required? This is a build-time tool, > > > > and Python gives nice stacktraces. However, segfaults are possible > > > > in Python version due to pointer handling. IMO, error checking > > > > must be just sufficient to prevent silent segfaults. > > > > > > > I'm not really sure what the question is here. You need to handle errors in the > > > parsing process, we can't just have the tool crashing during the build. How > > > thats handled is an implementation detail I would think. > > > > Consider a snippet from pmdinfogen.c: > > > > /* Check if file offset is correct */ > > if (hdr->e_shoff > info->size) { > > fprintf(stderr, "section header offset=%lu in file '%s' " > > "is bigger than filesize=%lu\n", > > (unsigned long)hdr->e_shoff, > > filename, info->size); > > return 0; > > } > > > > It is required in C, because otherwise pmdinfogen would crash without > > diagnostic. With Python, most errors like this result in a crash with a trace > > to the error line and a message from ctypes (or ELF parsing library). I'm > > arguing for leaving only the checks that prevent *silent* crashes (this can > > happen with ctypes) which are hard to diagnose. Motivation is to keep the > > code simple. > > > Hmm, I'd defer to others opinions on that. As a build tool it may be ok to just > crash with a backtrace, but I'm personally not a fan of that. Id rather see a > diagnostic error and have the tool exits with an appropriate exit code, but lets > see what others say. > I'd take a wait-and-see approach here. Obviously individualized errors messages are better, but for a case like the above where we have a malformed elf header, I would think that this is such a rare event that just letting python exception handling manage it should be ok. > > > > > > 3. On Unix, pmdinfogen is called for each object file extracted with ar > > > > from an .a library by a shell script. On Windows, other tools > > > > have to be used, shell script will not work. On the other hand, COFF > > > > library format is quite simple. Would it be appropriate for > > > > pmdinfogen to handle it to avoid intermediate script? > > > > > > > I suppose you could, but extracting objects from archives seems outside the > > > scope of what pmdinfogen normally does. What is the problem with using a shell > > > script on windows? I thought WSL had support for executing bash syntax shell > > > scripts there? Why not key off an environment variable to make the relevant > > > scripts do the right thing on your environment? > > > > WSL2 is a Linux VM integrated in Windows, you can only cross-compile from > > there. Native builds can use Python or PowerShell, which is as capable as Unix > > shells, but incompatible with them. To call lib.exe (MSVC analog of ar) is > > probably simpler then parsing COFF, yes. So meson could select one of the > > following for a Windows target (objects are COFF): > > > > Host Toolchain Archive Script Extractor > > ------- --------- ------- --------- --------- > > Linux MinGW .a sh ar > > Windows MinGW .a PowerShell ar > > Windows Clang .lib PowerShell lib > I think if I read you right, what you're suggesting here is that meson setup a > build time marco $ARCHIVETOOL, and set it to ar or lib.exe depending on the > build environment, and just have the script in question use $ARCHIVER? If so, > I'd be good with that. Do we have to worry about the use of divergent command > line options for each tool as well? > It might be better just to do that detection in the python script itself, rather than in meson, since the script may have to do other behavioural changes based on the platform. Regards, /Bruce