Re: [edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Fix VS2019 build error because of implicit cast

["Sean" <sean.brogan@microsoft.com>]

[-- Attachment #1: Type: text/plain, Size: 1289 bytes --]

I agree that safeintlib is not doing anything too interesting in this case but that's not really the point.  The argument for it is that it becomes the central point of code to check for safe conversions and an indicator that the developer was thoughtful about this conversion and didn't just cast to avoid the compiler complaining.  If everyone starts putting their own checks in place it leads to more code reviews, diversity in solutions, and opportunities for bugs.  All that said those are soft reasons for the change and that is up to you.

@Laszlo - On the ASSERT part, I have a different view point and am more curious about yours.  For release builds, I don't want to see CpuDeadLoops anywhere unless I am ok with the device being returned/refunded.  Our error path would be to exit the function with an error code and potentially log a ReportStatusCode.   I don't think you should continue in an invalid state as that just makes resolving the bug much much harder.    Given that the system can boot to at least a menu without this driver, it seems that failing out of the function would provide a better "RELEASE" experience.

Finally, given that this is contained in OVMF I am fine with whatever makes the most sense for your platform and usecase.

Thanks
Sean

[-- Attachment #2: Type: text/html, Size: 1372 bytes --]