From a973ca871b1c51b676a51041321b2e15da21ef34 Mon Sep 17 00:00:00 2001 From: badcel <1218031+badcel@users.noreply.github.com> Date: Thu, 12 Oct 2023 20:13:10 +0200 Subject: [PATCH] Prefer zero terminated string array over length based arrays. As zero terminated string arrays require a certain memory layout (NULL at the end) there could be a problem if a method has an array parameter which is zero terminated and length based at the same time. Preferring zero terminated rendering ensures that the memory layout fulfills the requirements. Additionally this fixes a bug that the zero terminated flag was never actively set as bool.TryParse can't parse "0" or "1" but the serializer does. --- .../Converter/PlatformStringArray.cs | 30 +++++++++---------- .../Parameter/Converter/Utf8StringArray.cs | 30 +++++++++---------- .../Converter/PlatformStringArray.cs | 7 +++-- .../Converter/Utf8StringArray.cs | 7 +++-- .../Converter/PlatformStringArray.cs | 20 ++++++++----- .../ReturnType/Converter/Utf8StringArray.cs | 20 ++++++++----- .../Converter/PlatformStringArray.cs | 7 +++-- .../Converter/Utf8StringArray.cs | 7 +++-- .../Converter/PlatformStringArray.cs | 12 ++++++-- .../Converter/Utf8StringArray.cs | 12 ++++++-- src/Generation/GirLoader/Input/ArrayType.cs | 2 +- .../GirLoader/Output/TypeReferenceFactory.cs | 9 ++---- 12 files changed, 96 insertions(+), 67 deletions(-) diff --git a/src/Generation/Generator/Renderer/Internal/Parameter/Converter/PlatformStringArray.cs b/src/Generation/Generator/Renderer/Internal/Parameter/Converter/PlatformStringArray.cs index 0baa4ed14..235c0350f 100644 --- a/src/Generation/Generator/Renderer/Internal/Parameter/Converter/PlatformStringArray.cs +++ b/src/Generation/Generator/Renderer/Internal/Parameter/Converter/PlatformStringArray.cs @@ -11,12 +11,18 @@ public bool Supports(GirModel.AnyType anyType) public RenderableParameter Convert(GirModel.Parameter parameter) { - return parameter.AnyTypeOrVarArgs.AsT0.AsT1.Length is null - ? ParameterWithoutLength(parameter) - : ParameterWithLength(parameter); + var arrayType = parameter.AnyTypeOrVarArgs.AsT0.AsT1; + + if (arrayType.IsZeroTerminated) + return NullTerminatedArray(parameter); + + if (arrayType.Length is not null) + return SizeBasedArray(parameter); + + throw new Exception("Unknown kind of array"); } - private static RenderableParameter ParameterWithLength(GirModel.Parameter parameter) + private static RenderableParameter SizeBasedArray(GirModel.Parameter parameter) { var length = parameter.AnyTypeOrVarArgs.AsT0.AsT1.Length ?? throw new Exception("Length must not be null"); @@ -28,21 +34,18 @@ private static RenderableParameter ParameterWithLength(GirModel.Parameter parame ); } - private static RenderableParameter ParameterWithoutLength(GirModel.Parameter parameter) + private static RenderableParameter NullTerminatedArray(GirModel.Parameter parameter) { return parameter.Direction switch { - GirModel.Direction.In => ParameterWithoutLengthIn(parameter), - GirModel.Direction.Out => ParameterWithoutLengthOut(parameter), + GirModel.Direction.In => NullTerminatedArrayIn(parameter), + GirModel.Direction.Out => NullTerminatedArrayOut(parameter), _ => throw new Exception("Unknown direction for parameter") }; } - private static RenderableParameter ParameterWithoutLengthIn(GirModel.Parameter parameter) + private static RenderableParameter NullTerminatedArrayIn(GirModel.Parameter parameter) { - if (parameter.AnyTypeOrVarArgs.AsT0.AsT1.Length is not null) - throw new Exception("Length must be null"); - if (parameter.Direction != GirModel.Direction.In) throw new Exception("Direction must be in"); @@ -62,11 +65,8 @@ private static RenderableParameter ParameterWithoutLengthIn(GirModel.Parameter p ); } - private static RenderableParameter ParameterWithoutLengthOut(GirModel.Parameter parameter) + private static RenderableParameter NullTerminatedArrayOut(GirModel.Parameter parameter) { - if (parameter.AnyTypeOrVarArgs.AsT0.AsT1.Length is not null) - throw new Exception("Length must be null"); - if (parameter.Direction != GirModel.Direction.Out) throw new Exception("Direction must be out"); diff --git a/src/Generation/Generator/Renderer/Internal/Parameter/Converter/Utf8StringArray.cs b/src/Generation/Generator/Renderer/Internal/Parameter/Converter/Utf8StringArray.cs index 606e6f863..815f59385 100644 --- a/src/Generation/Generator/Renderer/Internal/Parameter/Converter/Utf8StringArray.cs +++ b/src/Generation/Generator/Renderer/Internal/Parameter/Converter/Utf8StringArray.cs @@ -11,12 +11,18 @@ public bool Supports(GirModel.AnyType anyType) public RenderableParameter Convert(GirModel.Parameter parameter) { - return parameter.AnyTypeOrVarArgs.AsT0.AsT1.Length is null - ? ParameterWithoutLength(parameter) - : ParameterWithLength(parameter); + var arrayType = parameter.AnyTypeOrVarArgs.AsT0.AsT1; + + if (arrayType.IsZeroTerminated) + return NullTerminatedArray(parameter); + + if (arrayType.Length is not null) + return SizeBasedArray(parameter); + + throw new Exception("Unknown kind of array"); } - private static RenderableParameter ParameterWithLength(GirModel.Parameter parameter) + private static RenderableParameter SizeBasedArray(GirModel.Parameter parameter) { var length = parameter.AnyTypeOrVarArgs.AsT0.AsT1.Length ?? throw new Exception("Length must not be null"); @@ -28,21 +34,18 @@ private static RenderableParameter ParameterWithLength(GirModel.Parameter parame ); } - private static RenderableParameter ParameterWithoutLength(GirModel.Parameter parameter) + private static RenderableParameter NullTerminatedArray(GirModel.Parameter parameter) { return parameter.Direction switch { - GirModel.Direction.In => ParameterWithoutLengthIn(parameter), - GirModel.Direction.Out => ParameterWithoutLengthOut(parameter), + GirModel.Direction.In => NullTerminatedArrayIn(parameter), + GirModel.Direction.Out => NullTerminatedArrayOut(parameter), _ => throw new Exception("Unknown direction for parameter") }; } - private static RenderableParameter ParameterWithoutLengthIn(GirModel.Parameter parameter) + private static RenderableParameter NullTerminatedArrayIn(GirModel.Parameter parameter) { - if (parameter.AnyTypeOrVarArgs.AsT0.AsT1.Length is not null) - throw new Exception("Length must be null"); - if (parameter.Direction != GirModel.Direction.In) throw new Exception("Direction must be in"); @@ -62,11 +65,8 @@ private static RenderableParameter ParameterWithoutLengthIn(GirModel.Parameter p ); } - private static RenderableParameter ParameterWithoutLengthOut(GirModel.Parameter parameter) + private static RenderableParameter NullTerminatedArrayOut(GirModel.Parameter parameter) { - if (parameter.AnyTypeOrVarArgs.AsT0.AsT1.Length is not null) - throw new Exception("Length must be null"); - if (parameter.Direction != GirModel.Direction.Out) throw new Exception("Direction must be out"); diff --git a/src/Generation/Generator/Renderer/Internal/ParameterToManagedExpression/Converter/PlatformStringArray.cs b/src/Generation/Generator/Renderer/Internal/ParameterToManagedExpression/Converter/PlatformStringArray.cs index ec3ca885e..10bb37105 100644 --- a/src/Generation/Generator/Renderer/Internal/ParameterToManagedExpression/Converter/PlatformStringArray.cs +++ b/src/Generation/Generator/Renderer/Internal/ParameterToManagedExpression/Converter/PlatformStringArray.cs @@ -11,10 +11,13 @@ public bool Supports(GirModel.AnyType type) public void Initialize(ParameterToManagedData parameterData, IEnumerable parameters) { var arrayType = parameterData.Parameter.AnyTypeOrVarArgs.AsT0.AsT1; - if (arrayType.Length is null) + + if (arrayType.IsZeroTerminated) NullTerminatedArray(parameterData); - else + else if (arrayType.Length is not null) SizeBasedArray(parameterData); + else + throw new Exception("Unknown kind of array"); } private static void NullTerminatedArray(ParameterToManagedData parameterData) diff --git a/src/Generation/Generator/Renderer/Internal/ParameterToManagedExpression/Converter/Utf8StringArray.cs b/src/Generation/Generator/Renderer/Internal/ParameterToManagedExpression/Converter/Utf8StringArray.cs index 308aa5c1a..74da68a3e 100644 --- a/src/Generation/Generator/Renderer/Internal/ParameterToManagedExpression/Converter/Utf8StringArray.cs +++ b/src/Generation/Generator/Renderer/Internal/ParameterToManagedExpression/Converter/Utf8StringArray.cs @@ -11,10 +11,13 @@ public bool Supports(GirModel.AnyType type) public void Initialize(ParameterToManagedData parameterData, IEnumerable parameters) { var arrayType = parameterData.Parameter.AnyTypeOrVarArgs.AsT0.AsT1; - if (arrayType.Length is null) + + if (arrayType.IsZeroTerminated) NullTerminatedArray(parameterData); - else + else if (arrayType.Length is not null) SizeBasedArray(parameterData); + else + throw new Exception("Unknown kind of array"); } private static void NullTerminatedArray(ParameterToManagedData parameterData) diff --git a/src/Generation/Generator/Renderer/Internal/ReturnType/Converter/PlatformStringArray.cs b/src/Generation/Generator/Renderer/Internal/ReturnType/Converter/PlatformStringArray.cs index 4d2aaa0c2..a8457768c 100644 --- a/src/Generation/Generator/Renderer/Internal/ReturnType/Converter/PlatformStringArray.cs +++ b/src/Generation/Generator/Renderer/Internal/ReturnType/Converter/PlatformStringArray.cs @@ -13,26 +13,30 @@ public RenderableReturnType Convert(GirModel.ReturnType returnType) { var arrayType = returnType.AnyType.AsT1; - var nullableTypeName = arrayType.Length is null - ? NullTerminatedArray(returnType) - : SizeBasedArray(); + if (arrayType.IsZeroTerminated) + return NullTerminatedArray(returnType); - return new RenderableReturnType(nullableTypeName); + if (arrayType.Length is not null) + return SizeBasedArray(); + + throw new Exception("Unknown kind of array"); } - private static string NullTerminatedArray(GirModel.ReturnType returnType) + private static RenderableReturnType NullTerminatedArray(GirModel.ReturnType returnType) { - return returnType switch + var typeName = returnType switch { { Transfer: GirModel.Transfer.Full } => Model.PlatformStringArray.GetInternalOwnedHandleName(), { Transfer: GirModel.Transfer.None } => Model.PlatformStringArray.GetInternalUnownedHandleName(), { Transfer: GirModel.Transfer.Container } => Model.PlatformStringArray.GetInternalContainerHandleName(), _ => throw new Exception("Unknown transfer type for platform string array return value") }; + + return new RenderableReturnType(typeName); } - private static string SizeBasedArray() + private static RenderableReturnType SizeBasedArray() { - return "string[]"; + return new RenderableReturnType("string[]"); } } diff --git a/src/Generation/Generator/Renderer/Internal/ReturnType/Converter/Utf8StringArray.cs b/src/Generation/Generator/Renderer/Internal/ReturnType/Converter/Utf8StringArray.cs index 9f1421fd8..5b7cd51e6 100644 --- a/src/Generation/Generator/Renderer/Internal/ReturnType/Converter/Utf8StringArray.cs +++ b/src/Generation/Generator/Renderer/Internal/ReturnType/Converter/Utf8StringArray.cs @@ -13,26 +13,30 @@ public RenderableReturnType Convert(GirModel.ReturnType returnType) { var arrayType = returnType.AnyType.AsT1; - var nullableTypeName = arrayType.Length is null - ? NullTerminatedArray(returnType) - : SizeBasedArray(); + if (arrayType.IsZeroTerminated) + return NullTerminatedArray(returnType); - return new RenderableReturnType(nullableTypeName); + if (arrayType.Length is not null) + return SizeBasedArray(); + + throw new Exception("Unknown kind of array"); } - private static string NullTerminatedArray(GirModel.ReturnType returnType) + private static RenderableReturnType NullTerminatedArray(GirModel.ReturnType returnType) { - return returnType switch + var typeName = returnType switch { { Transfer: GirModel.Transfer.Full } => Model.Utf8StringArray.GetInternalOwnedHandleName(), { Transfer: GirModel.Transfer.None } => Model.Utf8StringArray.GetInternalUnownedHandleName(), { Transfer: GirModel.Transfer.Container } => Model.Utf8StringArray.GetInternalContainerHandleName(), _ => throw new Exception("Unknown transfer type for utf8 string array return value") }; + + return new RenderableReturnType(typeName); } - private static string SizeBasedArray() + private static RenderableReturnType SizeBasedArray() { - return "string[]"; + return new RenderableReturnType("string[]"); } } diff --git a/src/Generation/Generator/Renderer/Public/ParameterToNativeExpression/Converter/PlatformStringArray.cs b/src/Generation/Generator/Renderer/Public/ParameterToNativeExpression/Converter/PlatformStringArray.cs index 1a3bb93d3..e5251b161 100644 --- a/src/Generation/Generator/Renderer/Public/ParameterToNativeExpression/Converter/PlatformStringArray.cs +++ b/src/Generation/Generator/Renderer/Public/ParameterToNativeExpression/Converter/PlatformStringArray.cs @@ -11,10 +11,13 @@ public bool Supports(GirModel.AnyType type) public void Initialize(ParameterToNativeData parameter, IEnumerable _) { var arrayType = parameter.Parameter.AnyTypeOrVarArgs.AsT0.AsT1; - if (arrayType.Length == null) + + if (arrayType.IsZeroTerminated) NullTerminatedArray(parameter); - else + else if (arrayType.Length is not null) SizeBasedArray(parameter); + else + throw new Exception("Unknown kind of array"); } private static void NullTerminatedArray(ParameterToNativeData parameter) diff --git a/src/Generation/Generator/Renderer/Public/ParameterToNativeExpression/Converter/Utf8StringArray.cs b/src/Generation/Generator/Renderer/Public/ParameterToNativeExpression/Converter/Utf8StringArray.cs index 4658bcbc5..eb14b130a 100644 --- a/src/Generation/Generator/Renderer/Public/ParameterToNativeExpression/Converter/Utf8StringArray.cs +++ b/src/Generation/Generator/Renderer/Public/ParameterToNativeExpression/Converter/Utf8StringArray.cs @@ -11,10 +11,13 @@ public bool Supports(GirModel.AnyType type) public void Initialize(ParameterToNativeData parameter, IEnumerable _) { var arrayType = parameter.Parameter.AnyTypeOrVarArgs.AsT0.AsT1; - if (arrayType.Length is null) + + if (arrayType.IsZeroTerminated) NullTerminatedArray(parameter); - else + else if (arrayType.Length is not null) SizeBasedArray(parameter); + else + throw new Exception("Unknown kind of array"); } private static void NullTerminatedArray(ParameterToNativeData parameter) diff --git a/src/Generation/Generator/Renderer/Public/ReturnTypeToManagedExpression/Converter/PlatformStringArray.cs b/src/Generation/Generator/Renderer/Public/ReturnTypeToManagedExpression/Converter/PlatformStringArray.cs index 099c75650..d1a1fbf16 100644 --- a/src/Generation/Generator/Renderer/Public/ReturnTypeToManagedExpression/Converter/PlatformStringArray.cs +++ b/src/Generation/Generator/Renderer/Public/ReturnTypeToManagedExpression/Converter/PlatformStringArray.cs @@ -1,4 +1,6 @@ -namespace Generator.Renderer.Public.ReturnTypeToManagedExpressions; +using System; + +namespace Generator.Renderer.Public.ReturnTypeToManagedExpressions; internal class PlatformStringArray : ReturnTypeConverter { @@ -7,10 +9,14 @@ public bool Supports(GirModel.AnyType type) public string GetString(GirModel.ReturnType returnType, string fromVariableName) { - if (returnType.AnyType.AsT1.Length == null) + var arrayType = returnType.AnyType.AsT1; + if (arrayType.IsZeroTerminated) return NullTerminatedArray(returnType, fromVariableName); - else + + if (arrayType.Length is not null) return SizeBasedArray(returnType, fromVariableName); + + throw new Exception("Unknown kind of array"); } private static string NullTerminatedArray(GirModel.ReturnType returnType, string fromVariableName) diff --git a/src/Generation/Generator/Renderer/Public/ReturnTypeToManagedExpression/Converter/Utf8StringArray.cs b/src/Generation/Generator/Renderer/Public/ReturnTypeToManagedExpression/Converter/Utf8StringArray.cs index 062728f1c..21087eac8 100644 --- a/src/Generation/Generator/Renderer/Public/ReturnTypeToManagedExpression/Converter/Utf8StringArray.cs +++ b/src/Generation/Generator/Renderer/Public/ReturnTypeToManagedExpression/Converter/Utf8StringArray.cs @@ -1,4 +1,6 @@ -namespace Generator.Renderer.Public.ReturnTypeToManagedExpressions; +using System; + +namespace Generator.Renderer.Public.ReturnTypeToManagedExpressions; internal class Utf8StringArray : ReturnTypeConverter { @@ -7,10 +9,14 @@ public bool Supports(GirModel.AnyType type) public string GetString(GirModel.ReturnType returnType, string fromVariableName) { - if (returnType.AnyType.AsT1.Length == null) + var arrayType = returnType.AnyType.AsT1; + if (arrayType.IsZeroTerminated) return NullTerminatedArray(returnType, fromVariableName); - else + + if (arrayType.Length is not null) return SizeBasedArray(returnType, fromVariableName); + + throw new Exception("Unknown kind of array"); } private static string NullTerminatedArray(GirModel.ReturnType returnType, string fromVariableName) diff --git a/src/Generation/GirLoader/Input/ArrayType.cs b/src/Generation/GirLoader/Input/ArrayType.cs index 8c92fad82..7ab5b0650 100644 --- a/src/Generation/GirLoader/Input/ArrayType.cs +++ b/src/Generation/GirLoader/Input/ArrayType.cs @@ -11,7 +11,7 @@ public class ArrayType : AnyType public string? Length { get; set; } [XmlAttribute("zero-terminated")] - public string? ZeroTerminated { get; set; } + public bool ZeroTerminated { get; set; } [XmlAttribute("fixed-size")] public string? FixedSize { get; set; } diff --git a/src/Generation/GirLoader/Output/TypeReferenceFactory.cs b/src/Generation/GirLoader/Output/TypeReferenceFactory.cs index d8a14e330..83d9777d3 100644 --- a/src/Generation/GirLoader/Output/TypeReferenceFactory.cs +++ b/src/Generation/GirLoader/Output/TypeReferenceFactory.cs @@ -52,7 +52,6 @@ private bool TryCreateArrayTypeReference(Input.AnyType anyType, [NotNullWhen(tru int? length = int.TryParse(anyType.Array.Length, out var l) ? l : null; int? fixedSize = int.TryParse(anyType.Array.FixedSize, out var f) ? f : null; - bool? zeroTerminated = bool.TryParse(anyType.Array.ZeroTerminated, out var b) ? b : null; var reference = new ArrayTypeReference( typeReference: typeReference, @@ -61,11 +60,9 @@ private bool TryCreateArrayTypeReference(Input.AnyType anyType, [NotNullWhen(tru { Length = length, FixedSize = fixedSize, - // TODO: The zero-terminated attribute is not consistently present in the gir file. - // If there isn't a length and fixed size treat as zero-terminated anyways. The GLib - // array types are never zero-terminated and always reset the flag. - // This can be removed if the C gir generator changes this behavior. - IsZeroTerminated = zeroTerminated ?? (length is null && fixedSize is null) + //The fallback is required as gobject-introspection expects an array to be zero terminated, + //if neither length nor fixedSize are given. + IsZeroTerminated = anyType.Array.ZeroTerminated || (length is null && fixedSize is null) }; arrayTypeReference = anyType.Array.Name switch