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 9418BA0A0C; Thu, 1 Jul 2021 23:36:50 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 18D8E40141; Thu, 1 Jul 2021 23:36:50 +0200 (CEST) Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com [209.85.167.52]) by mails.dpdk.org (Postfix) with ESMTP id DBE964003E for ; Thu, 1 Jul 2021 23:36:48 +0200 (CEST) Received: by mail-lf1-f52.google.com with SMTP id t17so14480777lfq.0 for ; Thu, 01 Jul 2021 14:36:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=MshBDmKJdriKdQdr7RhEMF8Aj4fzpRY8pP2JatqvZ8c=; b=VOdT+8ZeVerAl/xyg/ktnevIbdmqrgDuB71P9WpSIlFx52Qtv8xMl4y7xog9txtwRU RaltndayMdPM/QZHSRXlIBzV/1zzaDYdNkoxvzcuvHoj+FyG6eQamOXkdHUQkNgbFb2s 6HS750JQKz75ia9pNmcvmy88aq+NiJ4Xia57MPomFB3pArMuWeEM0A6FWZslAoij86xJ ImvmxP3zGfPQvTp5BOpCisHnCc9J9pFfYKeQQgGYDL2X9oDoqK3tQllbu3PrAbzVKJkN 78DbQ6vXfR/lLVtIAYjJBFMUDsUmLDHbKneg3aXeTo7jKsdzDrAwJxJoM3xYrClczmHX s2zg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=MshBDmKJdriKdQdr7RhEMF8Aj4fzpRY8pP2JatqvZ8c=; b=KW8XgvyLz+lQFIJvpfQySjqv852+iaazW4CuM7yasXfm9LQcRytsD87aRx9UKdix0r Riw5DObXLN301fCzOtyPqvBG7eNhKTWZbMFmxEi1mpPulWr1OAFIqceuOtws+VGr7qV7 PD8GP84DEcidNM8FBbnp3noAjvUF8OtMZiKACIZiYYG24aUiqFaslTiPC01JJvpM7rPz /8F9kXSIiKr5EeFHgoYI1KlJfhX3PcuhCZtsjTNFCbNhK3/BqwfnUqxSF2B1uPj6kz5K pRRliGtOzCK/gfAZ/bvnEpo1ekVx6aMb8OKX4jD++oTZIe8s3m988+dNEaM01n6c5lml MnSQ== X-Gm-Message-State: AOAM531jqlASfRg9Pynoq5BR9vKo5J2xSq+Mvi3VFvUnCmPPKnF98oa8 +CIVBkrPZvObXt2x0chWxB4= X-Google-Smtp-Source: ABdhPJyJYw60YtPQWccNQfN9c2KalMn5mP+OpSEErbWLTDXQMeiIC80sm+cwaxnXi/rgyi4USHWgwg== X-Received: by 2002:ac2:546a:: with SMTP id e10mr1281109lfn.244.1625175408340; Thu, 01 Jul 2021 14:36:48 -0700 (PDT) Received: from sovereign (broadband-37-110-65-23.ip.moscow.rt.ru. [37.110.65.23]) by smtp.gmail.com with ESMTPSA id u19sm142593ljl.99.2021.07.01.14.36.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 01 Jul 2021 14:36:47 -0700 (PDT) Date: Fri, 2 Jul 2021 00:36:45 +0300 From: Dmitry Kozlyuk To: Tyler Retzlaff Cc: Jie Zhou , dev@dpdk.org, roretzla@microsoft.com, talshn@nvidia.com, pallavi.kadam@intel.com, navasile@linux.microsoft.com, dmitrym@microsoft.com Message-ID: <20210702003645.13813371@sovereign> In-Reply-To: <20210701162144.GA9873@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1624495001-16613-1-git-send-email-jizh@linux.microsoft.com> <20210701023129.2ee5e076@sovereign> <20210701162144.GA9873@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v1] lib/eal: enforce alarm APIs parameters check 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 Sender: "dev" 2021-07-01 09:21 (UTC-0700), Tyler Retzlaff: > On Thu, Jul 01, 2021 at 02:31:29AM +0300, Dmitry Kozlyuk wrote: > > Hi Jie, > > > > 2021-06-23 17:36 (UTC-0700), Jie Zhou: > > > From: Jie Zhou > [...] > > > + /* Check if us is valid */ > > > + if (us < 1 || us >(UINT64_MAX - US_PER_S)) { > > > > This condition is specific to Linux EAL. In fact, it's not very useful even > > there, because actual upper bound for `us` depends on current time. > > No bounds are specified in API description at all. > > Windows check would be different, but these considerations remain valid. > > > > Maybe it's alarm_autotest or API description that needs adjustments, > > but not the implementation. > > i agree with your assessment. it's a bit silly to test a range > constraint where the range is just some arbitrary range and not the real > range. changing the implementation to calculate the "real" valid range > isn't practical. i'm sure linux implementors would argue that catching > some extremely out of range values is better than none? Why we care about overflow? 1. It likely indicates a bug in the app. 2. Alarm is can to be scheduled to some time in the past and fire immediately, which means API did not do what it promises. I see the only way to tell the interval is incorrect if it gives (would give) time in the past when added to the current time. But this is an implementation detail and does not need testing. A separate patch can be submitted to change behavior for all OS. > > i guess a correct test would would calculate the valid range and test > against that but as per above the implementation won't pass the test. > > so we are left with > > * matching the range imposed in the linux implementation so we pass the > test as is. It would be the worst thing one could do, defying the purpose of unit tests. > * don't run the test with the input data that exercises this bogus range > constraint. > > i guess based on your comments you prefer the latter? so is our action > here to submit a patch for the test that doesn't test this range > conditionally on execenv windows? Yes, please remove this check from the test as it verifies no contract.