From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 47C3D45BFD; Mon, 28 Oct 2024 11:12:26 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1EED5402A3; Mon, 28 Oct 2024 11:12:26 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id A8E04400D7 for ; Mon, 28 Oct 2024 11:12:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1730110344; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=r8O6bsLA3U/C1iAKZVKpnm7A9opvIMO/QQlgfbfAAGg=; b=ceza2037r68OiysClMk5x/W8ws0J3WyxgKyNT5io3IJY0qSn+/XMpmy9QyW9sVqGQy+kIZ bC6HwnCsm4lAefOuZO/vwYioH/LlvyoTjjIGY0bwxkN6FhWr70xemVqx9jPKRPH9APsVwx bQgaWbqCEw264SJoThhjyQKedZMLrx0= Received: from mail-yw1-f199.google.com (mail-yw1-f199.google.com [209.85.128.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-90-OJC27T-2NHeSKzWRFQHYyw-1; Mon, 28 Oct 2024 06:12:23 -0400 X-MC-Unique: OJC27T-2NHeSKzWRFQHYyw-1 Received: by mail-yw1-f199.google.com with SMTP id 00721157ae682-6e38fabff35so79082237b3.0 for ; Mon, 28 Oct 2024 03:12:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730110342; x=1730715142; h=mime-version:user-agent:message-id:in-reply-to:date:references :organization:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=r8O6bsLA3U/C1iAKZVKpnm7A9opvIMO/QQlgfbfAAGg=; b=C4uqwJKEP9cpQ5oBVHKNRBNAlqLD68E0fiDDKfSzFjydFuFCFZAgfA5z1cxp38TeDC 4LUSCVAljfer6hyF2a6Kbc0hNLJgELT4tKZYZM0DBM3OA8eWu6xUGw741tac3XSdpHSx skYqshjJ0BExgeCt12SR5JxU6QLfn/7gmDMRGPzFj23UPgKstPdvDqoSHnwys9NaNZjo 7tkh186dCogyg+Omne57mqNretmpAX633eXszC82tHvaTkJ1zv+xDsiScC4WnS5E0qdV +R/VP503kdIUtnryKYdKiXcS320q5Qydv1ivnlfg/CI62YwmqlCCazOk66LsbAJt1l+s ryMw== X-Forwarded-Encrypted: i=1; AJvYcCXHOFeU3Ha/Rod0CAWF9URhOmZpEPWMW6J27G74A3XRktjaYFCSMSanp0rKfngd7++bjtI=@dpdk.org X-Gm-Message-State: AOJu0YyoGDkBKlwZ5MzDPkMzaG0rTqp3rmY2RPTE6hBqtijT6CNl0oOH nIVrhNpuiEXLwUfrV9MEiXa3V8hkG66aL6gRCbanCGtkyAb+8dhQ/lJXEliVClGYRgoaiuZyh2i i2iKTgM27EjHchdivK3k12Yf756ncO7RP9nVBz21x X-Received: by 2002:a05:6902:2b0d:b0:e29:1def:1032 with SMTP id 3f1490d57ef6-e3087bf83c5mr6336021276.41.1730110342557; Mon, 28 Oct 2024 03:12:22 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEM1tWZo1YScZ3GVRELs90R1oaOhy6jGd6dr2ZNQj9j4Xth+iz1f/xx/mP7IG9OraJ3Q2EU+w== X-Received: by 2002:a05:6902:2b0d:b0:e29:1def:1032 with SMTP id 3f1490d57ef6-e3087bf83c5mr6335992276.41.1730110342213; Mon, 28 Oct 2024 03:12:22 -0700 (PDT) Received: from localhost (88-120-130-27.subs.proxad.net. [88.120.130.27]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6d1799fc30fsm30689426d6.80.2024.10.28.03.12.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 28 Oct 2024 03:12:21 -0700 (PDT) Received: by localhost (Postfix, from userid 1000) id 48F9A5073C45; Mon, 28 Oct 2024 11:12:19 +0100 (CET) From: Dodji Seketeli To: Ferruh Yigit Cc: Akhil Goyal , dev@dpdk.org, thomas@monjalon.net, david.marchand@redhat.com, hemant.agrawal@nxp.com, anoobj@marvell.com, pablo.de.lara.guarch@intel.com, fiona.trahe@intel.com, declan.doherty@intel.com, matan@nvidia.com, g.singh@nxp.com, fanzhang.oss@gmail.com, jianjay.zhou@huawei.com, asomalap@amd.com, ruifeng.wang@arm.com, konstantin.v.ananyev@yandex.ru, radu.nicolau@intel.com, ajit.khaparde@broadcom.com, rnagadheeraj@marvell.com, mdr@ashroe.eu Subject: Re: [PATCH] [RFC] cryptodev: replace LIST_END enumerators with APIs Organization: Red Hat / France References: <20240905101438.3888274-1-gakhil@marvell.com> <10591cfb-d78e-4d50-9bb5-f6d1246662b3@amd.com> <87msjkdx7x.fsf@redhat.com> <39e9241d-d6d5-4e94-a3cb-506af805880d@amd.com> X-Operating-System: AlmaLinux 9.4 X-URL: http://www.redhat.com Date: Mon, 28 Oct 2024 11:12:19 +0100 In-Reply-To: <39e9241d-d6d5-4e94-a3cb-506af805880d@amd.com> (Ferruh Yigit's message of "Thu, 10 Oct 2024 01:35:34 +0100") Message-ID: <87v7xc35a4.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hello, Ferruh Yigit writes: [...] >> This change cause the value of the the FOOD_END enumerator to increase. >> And that increase might be problematic. At the moment, for it being >> problematic or not has to be the result of a careful review. >> > > As you said, FOOD_END value change can be sometimes problematic, but > sometimes it is not. > This what I referred as limitation that tool is not reporting only > problematic case, but require human review. Oh, I see. Thank you for clarifying. > (btw, this is a very useful tool, I don't want to sound like negative > about it, only want to address this recurring subject in dpdk.) No problem, I never assume you mean anything negative :-) [...] >> So, by default, abidiff will complain by saying that the value of >> FOO_END was changed. >> >> But you, as a community of practice, can decide that this kind of change >> to the value of the last enumerator is not a problematic change, after >> careful review of your code and practice. You thus can specify that >> the tool using a suppression specification which has the following >> syntax: >> >> [suppress_type] >> type_kind = enum >> changed_enumerators = FOO_END, ANOTHER_ENUM_END, AND_ANOTHER_ENUM_END >> >> or, alternatively, you can specify the enumerators you want to suppress >> the changes for as a list of regular expressions: >> >> [suppress_type] >> type_kind = enum >> changed_enumerators_regexp = .*_END$, .*_LAST$, .*_LIST_END$ >> >> Wouldn't that be enough to address your use case here (honest question)? >> > > We are already using suppress feature in dpdk. > > But difficulty is to decide if END (MAX) enum value change warning is an > actual ABI break or not. > > When tool gives warning, tendency is to just make warning go away, > mostly by removing END (MAX) enum without really analyzing if it is a > real ABI break. I see. [...] >>> [1] It would be better if tool gives END (MAX) enum value warnings only >>> if it is exchanged in an API, but not sure if this can be possible to >>> detect. >> >> I believe that if you want to know if an enumerator value is *USED* by a >> type (which I believe is at the root of what you are alluding to), then >> you would need a static analysis tool that works at the source level. >> Or, you need a human review of the code once the binary analysis tool >> told you that that value of the enumerator changed. >> >> Why ? please let me give you an example: >> >> enum foo_enum >> { >> FOO_FIRST, >> FOO_SECOND, >> FOO_END >> }; >> >> int array[FOO_END]; >> >> Once this is compiled into binary, what libabigail is going to see by >> analyzing the binary is that 'array' is an array of 2 integers. The >> information about the size of the array being initially an enumerator >> value is lost. To detect that, you need source level analysis. >> > > I see the problem. > > Is this the main reason that changing FOO_END value reported as warning? Yes, it is because of this class of issues. Actually if ANY enumerator value is changed, that is actually an ABI change. And that ABI change is either compatible or not. > If we forbid this kind of usage of the FOO_END, can we ignore this > warning safely? I would think so. But then, you'd have to also forbid the use of all enumerators, basically. I am not sure that would be practical. Rather I would tend to lean toward reviewing the use of enumerators, on a case by case basis, using tools like 'grep' and whatnot. What I would advise to forbid is the use of complicated macros or constructs that makes the review of the use of enumerators non-practical. You should be able to grep "FOO_END" and see where it's used in the source code. Reviewing that shouldn't take more than a few minutes whenever a tool warns about the change of its value. > >> But then, by reviewing the code, this is a construct that you can spot >> and allow or forbid, depending on your goals as a project. >> >> [...] >> >> Cheers, >> > -- Dodji