From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f41.google.com (mail-pj1-f41.google.com [209.85.216.41]) by mx.groups.io with SMTP id smtpd.web11.5501.1683585992964908678 for ; Mon, 08 May 2023 15:46:33 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="signature has expired" header.i=@gmail.com header.s=20221208 header.b=N4x4Vhv9; spf=pass (domain: gmail.com, ip: 209.85.216.41, mailfrom: pedro.falcato@gmail.com) Received: by mail-pj1-f41.google.com with SMTP id 98e67ed59e1d1-24ded4b33d7so3534165a91.3 for ; Mon, 08 May 2023 15:46:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1683585992; x=1686177992; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=UUWCFVT9EeGl9XmnoxfDcG8/XPvYL0PtzHlBQM9gryk=; b=N4x4Vhv9S49EP1s0eO+cHdkIPW6qSv7qav+xs1J5OCEhftzBN3GfOkuH2g/koKMCaV GhJJqZBJ7uqdmuLrprIAieMnU/S5GNLkg1QrkI/o3H8Urqg5FKBEMTblY146ntXNb6eR GEFQyNM5DGRgjrXhDJe12QN7TYF2eOGCRBeDp0R/SIFD8pmaLhDckAcjvhaMIGQueKIi lhPq1ioqL4ENe6SXRthF7ikK7TjSNgH4OZ37+ClBpZzIcgohJYHeBIDdOZQpB2sFtw2V R3+UVaQKu2m4B1CPb5wDsh/iQq3/9yAW1yLiXoVWYpyY3QDcv+E+e5I2x5wBmE7+PGsD RbJw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683585992; x=1686177992; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=UUWCFVT9EeGl9XmnoxfDcG8/XPvYL0PtzHlBQM9gryk=; b=dO++tr1m1m/aGPW94ciXmWuuJl94OTPHNnQufbRW2U+VOrod4V4elNeawMrLhR8nrn 1+gDUaxtiKcrmCxaPHI4zFYSfsBKmT5yoBs922kXBuTVW4LlXpJv0cWwQpINlm84KgLR WGwRvp2/wmF8wB+YuganNSUL+paPg/gzoAv2jVUQRYyg9yd2p6LerMPIhRxkRijoc60c tIhhWo+tcx4/CX2WwSNFM2YQjtDuQrY/0/Ru4liCu8OtdVSMvwfFIcKGCnZFkg5dJ8u9 I9A9p2r6/9TwETNUyD+2f0l/xXAle83TMUff5sDA340+CQro7z5GpLLrzh72jo5GLd0f Fpsg== X-Gm-Message-State: AC+VfDxMPxnOug+FTexotqMUqrhwQBsjKunno0kt0vZd4S80RbXI0ZLp iUTMKdKviYQhBxyQFJ7q47veqaZBnqlF00u59BI= X-Google-Smtp-Source: ACHHUZ6nbQZhi8nuSdFD514G5fC+jCf2xgtubnA1mYpZLwHSHgOQZCRPkHeRjWZPRfP+ZhI7BhcIMYP/Pnc3Mz3TIOo= X-Received: by 2002:a17:90a:9112:b0:24e:2d3:a404 with SMTP id k18-20020a17090a911200b0024e02d3a404mr12104250pjo.4.1683585992329; Mon, 08 May 2023 15:46:32 -0700 (PDT) MIME-Version: 1.0 References: <20230508215246.217002-1-pedro.falcato@gmail.com> <20230508215246.217002-2-pedro.falcato@gmail.com> In-Reply-To: From: "Pedro Falcato" Date: Mon, 8 May 2023 23:46:21 +0100 Message-ID: Subject: Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/SataControllerDxe: Remove useless null check To: Mike Maslenkin Cc: devel@edk2.groups.io, Jian J Wang , Liming Gao , Hao A Wu , Ray Ni Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, May 8, 2023 at 11:28=E2=80=AFPM Mike Maslenkin wrote: > > Hello Pedro, > Technically speaking ASSERT (Private !=3D NULL) doesn't cover this branc= h. > It should crash before as result of UninstallMultipleProtocolInterfaces()= call. > Obviously it make no sense in release target (under normal condition > when assertion is turned off), while this code does. > But I would suggest to remove ASSERT (Private !=3D NULL) as well since > it is useless also. > It needs to be very lucky to get NULL as result of BASE_CR(), but > actually SATA_CONTROLLER_PRIVATE_DATA_FROM_THIS() and CR() > definition care about this. There will be assert if signature doesn't > match to dereferenced memory area before Private !=3D NULL check. > > In fact, this patch just reduces indentation level by removing useless ch= ecks. Hmm yes, I agree. I don't see how that ASSERT can realistically fire. It should also be removed. I'm keeping this patch as-is since it's essentially a cheap backport from OVMF SataControllerDxe; maintainers, pull it if you'd like, or I can send a v2 later on that cleans up that ASSERT as well. The OVMF patch doesn't really depend on this, as this is just refactoring. (sidenote: you're dropping all CCs accidentally) --=20 Pedro