From 2c955cfc4986f930d7bc434a62f2da4a724814a7 Mon Sep 17 00:00:00 2001 From: Natalia Portillo Date: Thu, 5 Oct 2023 02:19:02 +0100 Subject: [PATCH] [Aaru.Core] Improve null safety Several files in the Aaru.Core project have been updated to improve null safety. The modification of these files specifically handled null occurrences. Nullable value types are now correctly handled and default values are set to be used where nulls were previously unhandled. This will help prevent null reference exceptions and improve the overall stability of the code. --- Aaru.Core/Devices/Dumping/ATA.cs | 3 +- Aaru.Core/Devices/Dumping/MiniDisc.cs | 2 +- Aaru.Core/Devices/Dumping/SSC.cs | 52 +++++++++++----------- Aaru.Core/Devices/Dumping/Sbc/Dump.cs | 12 ++--- Aaru.Core/Devices/Dumping/XGD.cs | 3 +- Aaru.Core/Devices/Report/GdRomSwapTrick.cs | 6 ++- Aaru.Core/Devices/Report/MMC.cs | 4 +- Aaru.Core/Devices/Report/SSC.cs | 2 +- Aaru.Tests.Devices/SCSI_MMC/GdRom.cs | 14 +++--- Aaru.Tests/Issues/FsExtractIssueTest.cs | 2 +- Aaru/Commands/Device/DeviceReport.cs | 38 ++++++++++------ 11 files changed, 77 insertions(+), 61 deletions(-) diff --git a/Aaru.Core/Devices/Dumping/ATA.cs b/Aaru.Core/Devices/Dumping/ATA.cs index 7f27d4503..b7f4e6ea0 100644 --- a/Aaru.Core/Devices/Dumping/ATA.cs +++ b/Aaru.Core/Devices/Dumping/ATA.cs @@ -96,7 +96,8 @@ public partial class Dump if(ataIdNullable != null) { - Identify.IdentifyDevice ataId = ataIdNullable.Value; + // Guaranteed to never fall into default + Identify.IdentifyDevice ataId = ataIdNullable ?? default(Identify.IdentifyDevice); byte[] ataIdentify = cmdBuf; double totalDuration = 0; double currentSpeed = 0; diff --git a/Aaru.Core/Devices/Dumping/MiniDisc.cs b/Aaru.Core/Devices/Dumping/MiniDisc.cs index cce3e56e6..1f99af5c1 100644 --- a/Aaru.Core/Devices/Dumping/MiniDisc.cs +++ b/Aaru.Core/Devices/Dumping/MiniDisc.cs @@ -93,7 +93,7 @@ partial class Dump decMode = Modes.DecodeMode6(cmdBuf, _dev.ScsiType); if(decMode.HasValue) - scsiMediumType = (byte)decMode.Value.Header.MediumType; + scsiMediumType = (byte)(decMode?.Header.MediumType ?? default(MediumTypes)); if(blockSize != 2048) { diff --git a/Aaru.Core/Devices/Dumping/SSC.cs b/Aaru.Core/Devices/Dumping/SSC.cs index 1dfb803cc..e87d04aca 100644 --- a/Aaru.Core/Devices/Dumping/SSC.cs +++ b/Aaru.Core/Devices/Dumping/SSC.cs @@ -78,23 +78,23 @@ partial class Dump InitProgress?.Invoke(); - if(decSense.HasValue && decSense.Value.SenseKey != SenseKeys.NoSense) + if(decSense.HasValue && decSense?.SenseKey != SenseKeys.NoSense) { - _dumpLog.WriteLine(Localization.Core.Device_not_ready_Sense, decSense.Value.SenseKey, decSense.Value.ASC, - decSense.Value.ASCQ); + _dumpLog.WriteLine(Localization.Core.Device_not_ready_Sense, decSense?.SenseKey, decSense?.ASC, + decSense?.ASCQ); StoppingErrorMessage?.Invoke(Localization.Core.Drive_has_status_error_please_correct_Sense_follows + Environment.NewLine + - decSense.Value.Description); + decSense?.Description); return; } // Not in BOM/P - if(decSense is { ASC: 0x00 } && - decSense.Value.ASCQ != 0x00 && - decSense.Value.ASCQ != 0x04 && - decSense.Value.SenseKey != SenseKeys.IllegalRequest) + if(decSense is { ASC: 0x00 } && + decSense?.ASCQ != 0x00 && + decSense?.ASCQ != 0x04 && + decSense?.SenseKey != SenseKeys.IllegalRequest) { _dumpLog.WriteLine(Localization.Core.Rewinding_please_wait); PulseProgress?.Invoke(Localization.Core.Rewinding_please_wait); @@ -116,17 +116,16 @@ partial class Dump // And yet, did not rewind! if(decSense.HasValue && - (decSense.Value.ASC == 0x00 && decSense.Value.ASCQ != 0x04 && decSense.Value.ASCQ != 0x00 || - decSense.Value.ASC != 0x00)) + (decSense?.ASC == 0x00 && decSense?.ASCQ != 0x04 && decSense?.ASCQ != 0x00 || decSense?.ASC != 0x00)) { StoppingErrorMessage?.Invoke(Localization.Core.Drive_could_not_rewind_please_correct_Sense_follows + Environment.NewLine + - decSense.Value.Description); + decSense?.Description); _dumpLog.WriteLine(Localization.Core.Drive_could_not_rewind_please_correct_Sense_follows); - _dumpLog.WriteLine(Localization.Core.Device_not_ready_Sense, decSense.Value.SenseKey, - decSense.Value.ASC, decSense.Value.ASCQ); + _dumpLog.WriteLine(Localization.Core.Device_not_ready_Sense, decSense?.SenseKey, decSense?.ASC, + decSense?.ASCQ); return; } @@ -142,17 +141,17 @@ partial class Dump decSense = Sense.Decode(senseBuf); if(decSense.HasValue && - (decSense.Value.ASC == 0x20 && decSense.Value.ASCQ != 0x00 || - decSense.Value.ASC != 0x20 && decSense.Value.SenseKey != SenseKeys.IllegalRequest)) + (decSense?.ASC == 0x20 && decSense?.ASCQ != 0x00 || + decSense?.ASC != 0x20 && decSense?.SenseKey != SenseKeys.IllegalRequest)) { StoppingErrorMessage?.Invoke(Localization.Core.Could_not_get_position_Sense_follows + Environment.NewLine + - decSense.Value.Description); + decSense?.Description); _dumpLog.WriteLine(Localization.Core.Could_not_get_position_Sense_follows); - _dumpLog.WriteLine(Localization.Core.Device_not_ready_Sense, decSense.Value.SenseKey, - decSense.Value.ASC, decSense.Value.ASCQ); + _dumpLog.WriteLine(Localization.Core.Device_not_ready_Sense, decSense?.SenseKey, decSense?.ASC, + decSense?.ASCQ); return; } @@ -194,17 +193,16 @@ partial class Dump // And yet, did not rewind! if(decSense.HasValue && - (decSense.Value.ASC == 0x00 && decSense.Value.ASCQ != 0x04 && decSense.Value.ASCQ != 0x00 || - decSense.Value.ASC != 0x00)) + (decSense?.ASC == 0x00 && decSense?.ASCQ != 0x04 && decSense?.ASCQ != 0x00 || decSense?.ASC != 0x00)) { StoppingErrorMessage?.Invoke(Localization.Core.Drive_could_not_rewind_please_correct_Sense_follows + Environment.NewLine + - decSense.Value.Description); + decSense?.Description); _dumpLog.WriteLine(Localization.Core.Drive_could_not_rewind_please_correct_Sense_follows); - _dumpLog.WriteLine(Localization.Core.Device_not_ready_Sense, decSense.Value.SenseKey, - decSense.Value.ASC, decSense.Value.ASCQ); + _dumpLog.WriteLine(Localization.Core.Device_not_ready_Sense, decSense?.SenseKey, decSense?.ASC, + decSense?.ASCQ); return; } @@ -289,12 +287,12 @@ partial class Dump // TODO: Check partitions page if(decMode.HasValue) { - scsiMediumTypeTape = (byte)decMode.Value.Header.MediumType; + scsiMediumTypeTape = (byte)(decMode?.Header.MediumType ?? default(MediumTypes)); - if(decMode.Value.Header.BlockDescriptors?.Length > 0) - scsiDensityCodeTape = (byte)decMode.Value.Header.BlockDescriptors[0].Density; + if(decMode?.Header.BlockDescriptors?.Length > 0) + scsiDensityCodeTape = (byte)(decMode?.Header.BlockDescriptors[0].Density ?? default(DensityType)); - blockSize = decMode.Value.Header.BlockDescriptors?[0].BlockLength ?? 0; + blockSize = decMode?.Header.BlockDescriptors?[0].BlockLength ?? 0; UpdateStatus?.Invoke(string.Format(Localization.Core.Device_reports_0_blocks, blocks)); } diff --git a/Aaru.Core/Devices/Dumping/Sbc/Dump.cs b/Aaru.Core/Devices/Dumping/Sbc/Dump.cs index 33809ddd5..067cdb004 100644 --- a/Aaru.Core/Devices/Dumping/Sbc/Dump.cs +++ b/Aaru.Core/Devices/Dumping/Sbc/Dump.cs @@ -176,15 +176,15 @@ partial class Dump if(decMode.HasValue) { - scsiMediumType = (byte)decMode.Value.Header.MediumType; + scsiMediumType = (byte)(decMode?.Header.MediumType ?? default(MediumTypes)); - if(decMode.Value.Header.BlockDescriptors?.Length > 0) - scsiDensityCode = (byte)decMode.Value.Header.BlockDescriptors[0].Density; + if(decMode?.Header.BlockDescriptors?.Length > 0) + scsiDensityCode = (byte)(decMode?.Header.BlockDescriptors[0].Density ?? default(DensityType)); // TODO: Fix this - containsFloppyPage = decMode.Value.Pages?.Aggregate(containsFloppyPage, - (current, modePage) => - current | modePage.Page == 0x05) == + containsFloppyPage = decMode?.Pages?.Aggregate(containsFloppyPage, + (current, modePage) => + current | modePage.Page == 0x05) == true; } } diff --git a/Aaru.Core/Devices/Dumping/XGD.cs b/Aaru.Core/Devices/Dumping/XGD.cs index 1c1fe2a61..6c7a50c56 100644 --- a/Aaru.Core/Devices/Dumping/XGD.cs +++ b/Aaru.Core/Devices/Dumping/XGD.cs @@ -356,7 +356,8 @@ partial class Dump return; } - PFI.PhysicalFormatInformation wxRipperPfi = wxRipperPfiNullable.Value; + // Guaranteed to never fall into default + PFI.PhysicalFormatInformation wxRipperPfi = wxRipperPfiNullable ?? default(PFI.PhysicalFormatInformation); UpdateStatus?.Invoke(string.Format(Localization.Core.WxRipper_PFI_Data_Area_Start_PSN_0_sectors, wxRipperPfi.DataAreaStartPSN)); diff --git a/Aaru.Core/Devices/Report/GdRomSwapTrick.cs b/Aaru.Core/Devices/Report/GdRomSwapTrick.cs index 6717f0283..6218ffd2e 100644 --- a/Aaru.Core/Devices/Report/GdRomSwapTrick.cs +++ b/Aaru.Core/Devices/Report/GdRomSwapTrick.cs @@ -126,7 +126,8 @@ public sealed partial class DeviceReport return; } - FullTOC.CDFullTOC toc = decodedToc.Value; + // Guaranteed to never fall into default + FullTOC.CDFullTOC toc = decodedToc ?? default(FullTOC.CDFullTOC); FullTOC.TrackDataDescriptor leadOutTrack = toc.TrackDescriptors.FirstOrDefault(t => t.POINT == 0xA2); @@ -241,7 +242,8 @@ public sealed partial class DeviceReport return; } - toc = decodedToc.Value; + // Guaranteed to never fall into default + toc = decodedToc ?? default(FullTOC.CDFullTOC); FullTOC.TrackDataDescriptor newLeadOutTrack = toc.TrackDescriptors.FirstOrDefault(t => t.POINT == 0xA2); diff --git a/Aaru.Core/Devices/Report/MMC.cs b/Aaru.Core/Devices/Report/MMC.cs index 6538f107e..483f5de31 100644 --- a/Aaru.Core/Devices/Report/MMC.cs +++ b/Aaru.Core/Devices/Report/MMC.cs @@ -1373,7 +1373,7 @@ public sealed partial class DeviceReport if(mediaType == "Audio CD") { mediaTest.CanReadLeadOut = !_dev.ReadCd(out buffer, out senseBuffer, - (uint)(mediaTest.Blocks + 1), 2352, 1, + (uint)((mediaTest.Blocks ?? 0) + 1), 2352, 1, MmcSectorTypes.Cdda, false, false, false, MmcHeaderCodes.None, true, false, MmcErrorField.None, MmcSubchannel.None, _dev.Timeout, out _); @@ -1381,7 +1381,7 @@ public sealed partial class DeviceReport else { mediaTest.CanReadLeadOut = !_dev.ReadCd(out buffer, out senseBuffer, - (uint)(mediaTest.Blocks + 1), 2352, 1, + (uint)((mediaTest.Blocks ?? 0) + 1), 2352, 1, MmcSectorTypes.AllTypes, false, false, true, MmcHeaderCodes.AllHeaders, true, true, MmcErrorField.None, MmcSubchannel.None, _dev.Timeout, diff --git a/Aaru.Core/Devices/Report/SSC.cs b/Aaru.Core/Devices/Report/SSC.cs index 059266744..afcb823f6 100644 --- a/Aaru.Core/Devices/Report/SSC.cs +++ b/Aaru.Core/Devices/Report/SSC.cs @@ -198,7 +198,7 @@ public sealed partial class DeviceReport seqTest.MediumType = (byte)decMode.Value.Header.MediumType; if(decMode.Value.Header.BlockDescriptors?.Length > 0) - seqTest.Density = (byte)decMode.Value.Header.BlockDescriptors?[0].Density; + seqTest.Density = (byte)(decMode.Value.Header.BlockDescriptors?[0].Density ?? default(DensityType)); } Spectre.ProgressSingleSpinner(ctx => diff --git a/Aaru.Tests.Devices/SCSI_MMC/GdRom.cs b/Aaru.Tests.Devices/SCSI_MMC/GdRom.cs index b9701718b..c415bd40e 100644 --- a/Aaru.Tests.Devices/SCSI_MMC/GdRom.cs +++ b/Aaru.Tests.Devices/SCSI_MMC/GdRom.cs @@ -42,10 +42,10 @@ static partial class ScsiMmc DecodedSense? decodedSense = Sense.Decode(senseBuffer); - if(decodedSense.Value.ASC != 0x04) + if(decodedSense?.ASC != 0x04) break; - if(decodedSense.Value.ASCQ != 0x01) + if(decodedSense?.ASCQ != 0x01) break; Thread.Sleep(2000); @@ -74,7 +74,8 @@ static partial class ScsiMmc return; } - FullTOC.CDFullTOC toc = decodedToc.Value; + // Guaranteed to never fall into default + FullTOC.CDFullTOC toc = decodedToc ?? default(FullTOC.CDFullTOC); FullTOC.TrackDataDescriptor leadOutTrack = toc.TrackDescriptors.FirstOrDefault(t => t.POINT == 0xA2); @@ -153,10 +154,10 @@ static partial class ScsiMmc DecodedSense? decodedSense = Sense.Decode(senseBuffer); - if(decodedSense.Value.ASC != 0x04) + if(decodedSense?.ASC != 0x04) break; - if(decodedSense.Value.ASCQ != 0x01) + if(decodedSense?.ASCQ != 0x01) break; } while(retries < 25); @@ -181,7 +182,8 @@ static partial class ScsiMmc return; } - toc = decodedToc.Value; + // Guaranteed to never fall into default + toc = decodedToc ?? default; FullTOC.TrackDataDescriptor newLeadOutTrack = toc.TrackDescriptors.FirstOrDefault(t => t.POINT == 0xA2); diff --git a/Aaru.Tests/Issues/FsExtractIssueTest.cs b/Aaru.Tests/Issues/FsExtractIssueTest.cs index b720c6c06..8bc542d0c 100644 --- a/Aaru.Tests/Issues/FsExtractIssueTest.cs +++ b/Aaru.Tests/Issues/FsExtractIssueTest.cs @@ -109,7 +109,7 @@ public abstract class FsExtractIssueTest var fs = Activator.CreateInstance(pluginType) as IReadOnlyFilesystem; - Assert.IsNotNull(fs, string.Format(Localization.Could_not_instantiate_filesystem_0, fs.Name)); + Assert.IsNotNull(fs, string.Format(Localization.Could_not_instantiate_filesystem_0, fs?.Name)); filesystemFound = true; diff --git a/Aaru/Commands/Device/DeviceReport.cs b/Aaru/Commands/Device/DeviceReport.cs index fdd838eb0..e97f1fbbb 100644 --- a/Aaru/Commands/Device/DeviceReport.cs +++ b/Aaru/Commands/Device/DeviceReport.cs @@ -895,7 +895,7 @@ sealed class DeviceReportCommand : Command task.MaxValue = ushort.MaxValue; - for(var i = (ushort)mediaTest.BlockSize;; i++) + for(var i = (ushort)(mediaTest.BlockSize ?? 0);; i++) { task.Description = string. @@ -933,7 +933,9 @@ sealed class DeviceReportCommand : Command ctx.AddTask(Localization.Core.Trying_SCSI_READ_LONG_10).IsIndeterminate(); sense = dev.ReadLong10(out buffer, out senseBuffer, false, false, 0, - (ushort)mediaTest.LongBlockSize, dev.Timeout, out _); + (ushort)(mediaTest.LongBlockSize ?? + mediaTest.BlockSize ?? 0), dev.Timeout, + out _); }); if(!sense) @@ -948,7 +950,8 @@ sealed class DeviceReportCommand : Command ctx.AddTask(Localization.Core.Trying_SCSI_READ_LONG_16).IsIndeterminate(); sense = dev.ReadLong16(out buffer, out senseBuffer, false, 0, - mediaTest.LongBlockSize.Value, dev.Timeout, out _); + mediaTest.LongBlockSize ?? mediaTest.BlockSize ?? 0, + dev.Timeout, out _); }); if(!sense) @@ -1270,7 +1273,7 @@ sealed class DeviceReportCommand : Command task.MaxValue = ushort.MaxValue; - for(var i = (ushort)mediaTest.BlockSize;; i++) + for(var i = (ushort)(mediaTest.BlockSize ?? 0);; i++) { task.Value = i; @@ -1309,7 +1312,8 @@ sealed class DeviceReportCommand : Command ctx.AddTask(Localization.Core.Trying_SCSI_READ_LONG_10).IsIndeterminate(); sense = dev.ReadLong10(out buffer, out senseBuffer, false, false, 0, - (ushort)mediaTest.LongBlockSize, dev.Timeout, out _); + (ushort)(mediaTest.LongBlockSize ?? + mediaTest.BlockSize ?? 0), dev.Timeout, out _); }); if(!sense) @@ -1324,7 +1328,8 @@ sealed class DeviceReportCommand : Command ctx.AddTask(Localization.Core.Trying_SCSI_READ_LONG_16).IsIndeterminate(); sense = dev.ReadLong16(out buffer, out senseBuffer, false, 0, - (ushort)mediaTest.LongBlockSize, dev.Timeout, out _); + (ushort)(mediaTest.LongBlockSize ?? + mediaTest.BlockSize ?? 0), dev.Timeout, out _); }); if(!sense) @@ -1464,7 +1469,7 @@ sealed class DeviceReportCommand : Command task.MaxValue = ushort.MaxValue; - for(var i = (ushort)mediaTest.BlockSize;; i++) + for(var i = (ushort)(mediaTest.BlockSize ?? 0);; i++) { task.Value = i; @@ -1504,7 +1509,9 @@ sealed class DeviceReportCommand : Command ctx.AddTask(Localization.Core.Trying_SCSI_READ_LONG_10).IsIndeterminate(); sense = dev.ReadLong10(out buffer, out senseBuffer, false, false, 0, - (ushort)mediaTest.LongBlockSize, dev.Timeout, out _); + (ushort)(mediaTest.LongBlockSize ?? + mediaTest.BlockSize ?? 0), dev.Timeout, + out _); }); if(!sense) @@ -1519,7 +1526,9 @@ sealed class DeviceReportCommand : Command ctx.AddTask(Localization.Core.Trying_SCSI_READ_LONG_16).IsIndeterminate(); sense = dev.ReadLong16(out buffer, out senseBuffer, false, 0, - (ushort)mediaTest.LongBlockSize, dev.Timeout, out _); + (ushort)(mediaTest.LongBlockSize ?? + mediaTest.BlockSize ?? 0), dev.Timeout, + out _); }); if(!sense) @@ -1562,7 +1571,8 @@ sealed class DeviceReportCommand : Command task.MaxValue = ushort.MaxValue; - for(var i = (ushort)report.SCSI.ReadCapabilities.BlockSize;; i++) + for(var i = (ushort)(report.SCSI.ReadCapabilities.BlockSize ?? 0);; + i++) { task.Value = i; @@ -1603,7 +1613,8 @@ sealed class DeviceReportCommand : Command ctx.AddTask(Localization.Core.Trying_SCSI_READ_LONG_10).IsIndeterminate(); sense = dev.ReadLong10(out buffer, out senseBuffer, false, false, 0, - (ushort)report.SCSI.ReadCapabilities.LongBlockSize, + (ushort)(report.SCSI.ReadCapabilities.LongBlockSize ?? + report.SCSI.ReadCapabilities.BlockSize ?? 0), dev.Timeout, out _); }); @@ -1619,8 +1630,9 @@ sealed class DeviceReportCommand : Command ctx.AddTask(Localization.Core.Trying_SCSI_READ_LONG_16).IsIndeterminate(); sense = dev.ReadLong16(out buffer, out senseBuffer, false, 0, - report.SCSI.ReadCapabilities.LongBlockSize.Value, - dev.Timeout, out _); + report.SCSI.ReadCapabilities.LongBlockSize ?? + report.SCSI.ReadCapabilities.BlockSize ?? 0, dev.Timeout, + out _); }); if(!sense)