Phreak87 / LeptonicaSharp

Full featured wrapper for leptonica 1.77.0
Other
8 stars 5 forks source link

Binary Write Bug #47

Closed fdncred closed 5 years ago

fdncred commented 5 years ago

This code is not correct.

  1. If I'm writing a file, the library shouldn't have to check to see if that file exists and then fail if it does not exist.
  2. The data parameter is never being used. In my instance data is a byte[] but it may not always be a byte[]. I'm not sure how to change your code to make it work.

    Public Shared Function l_binaryWrite(
                 ByVal filename As String,
                 ByVal operation As String,
                 ByVal data As Object,
                 ByVal nbytes As UInteger) As Integer
    
        If IsNothing(filename) Then Throw New ArgumentNullException("filename cannot be Nothing")
        If IsNothing(operation) Then Throw New ArgumentNullException("operation cannot be Nothing")
        If IsNothing(data) Then Throw New ArgumentNullException("data cannot be Nothing")
    
        If My.Computer.Filesystem.Fileexists (filename) = false then Throw New ArgumentException ("File is missing")
    
        Dim dataPTR As IntPtr = Marshal.AllocHGlobal(0)
    
        Dim _Result As Integer = LeptonicaSharp.Natives.l_binaryWrite(filename, operation, dataPTR, nbytes)
    
        Return _Result
    End Function
fdncred commented 5 years ago

Here's my changes to the library and the function I used to test it. Changes to Library:

    Public Shared Function l_binaryWrite(
                 ByVal filename As String,
                 ByVal operation As String,
                 ByVal data As Object,
                 ByVal nbytes As UInteger) As Integer

        If IsNothing(filename) Then Throw New ArgumentNullException("filename cannot be Nothing")
        If IsNothing(operation) Then Throw New ArgumentNullException("operation cannot be Nothing")
        If IsNothing(data) Then Throw New ArgumentNullException("data cannot be Nothing")

        'If My.Computer.Filesystem.Fileexists (filename) = false then Throw New ArgumentException ("File is missing")
        Dim dataPtr As IntPtr
        If TypeOf data Is Byte() Then
            Dim cdata = CType(data, Byte())
            dataPtr = Marshal.AllocHGlobal(cdata.Length)
            Marshal.Copy(cdata, 0, dataPtr, cdata.Length)
        Else
            dataPtr = Marshal.AllocHGlobal(0)
        End If
        Dim _Result As Integer = LeptonicaSharp.Natives.l_binaryWrite(filename, operation, dataPTR, nbytes)
        Marshal.FreeHGlobal(dataPTR)

        Return _Result
    End Function

Test Function:

        private void TestArrayFunctions()
        {
            var feyn = Path.GetFullPath(@"..\..\..\..\ALL_Images\Leptonica\feyn.tif");
            var test24 = Path.GetFullPath(@"..\..\..\..\ALL_Images\Leptonica\test24.jpg");

            var lba1 = l_byteaInitFromFile(feyn);
            var lba2 = l_byteaInitFromFile(test24);
            var size1 = l_byteaGetSize(lba1);
            var size2 = l_byteaGetSize(lba2);
            l_byteaJoin(lba1, ref lba2);
            var lba3 = l_byteaInitFromMem(lba1.data, size1);
            //var lba4 = l_byteaInitFromMem(lba1.data + size1, size2)
            var lba4 = l_byteaInitFromMem(lba1.data.Skip((int)size1).ToArray(), size2);

            l_binaryWrite("junk1.dat", "w", lba3.data, lba3.size);
            l_binaryWrite("junk2.dat", "w", lba4.data, lba4.size);
            filesAreIdentical(feyn, "junk1.dat", out int psame1);
            filesAreIdentical(test24, "junk2.dat", out int psame2);
        }
Phreak87 commented 5 years ago

I Implemented this Version:

    Dim dataPtr As IntPtr = IntPtr.Zero
    If TypeOf (data) Is IntPtr Then dataPtr = data
    If TypeOf (data) Is Byte() Then
        Dim cdata = CType(data, Byte())
        dataPtr = Marshal.AllocHGlobal(cdata.Length - 1)
        Marshal.Copy(cdata, 0, dataPtr, cdata.Length)
    End If
    If Not IsNothing(data.GetType.GetProperty("data")) Then
        Dim cdata = CType(data.data, Byte())
        dataPtr = Marshal.AllocHGlobal(cdata.Length - 1)
        Marshal.Copy(cdata, 0, dataPtr, cdata.Length)
    End If

This allows to do the following and is the best way to handle all types with Object as type:

    Dim x1= LeptonicaSharp._All.l_binaryWrite("Test", "w", New Byte() {1, 2, 2}, 3)
    Dim x2 = LeptonicaSharp._All.l_binaryWrite("Test", "w", Pix, Pix.data.Length)
    Dim x3 = LeptonicaSharp._All.l_binaryWrite("Test", "w", Pix.Pointer, Pix.data.length)

For this and all others i need to implement FreeHAlloc too - Good Point

Phreak87 commented 5 years ago

If you see allocations stay forever please give me a short feedback!

fdncred commented 5 years ago

Changes got overwritten - reopening.

fdncred commented 5 years ago

C# Version of you VB.Net with a slight change. I added an elses since there is not reason to check the other data types if one of the first ones match.

public static int l_binaryWrite(
                String filename,
                String operation,
                Object data,
                uint nbytes)
{
    if (filename == null) {throw new ArgumentNullException  ("filename cannot be Nothing");}

    if (operation == null) {throw new ArgumentNullException  ("operation cannot be Nothing");}

    if (data == null) {throw new ArgumentNullException  ("data cannot be Nothing");}

    IntPtr dataPtr = IntPtr.Zero;
    if (data.GetType() == typeof(IntPtr))
        dataPtr = (IntPtr)data;
    else if (data.GetType() == typeof(Byte[]))
    {
        var cdata = (Byte[])data;
        dataPtr = Marshal.AllocHGlobal(cdata.Length);
        Marshal.Copy(cdata, 0, dataPtr, cdata.Length);
    }
    else if (data.GetType().GetProperty("data") != null)
    {
        var cdata = (Byte[])data;
        dataPtr = Marshal.AllocHGlobal(cdata.Length);
        Marshal.Copy(cdata, 0, dataPtr, cdata.Length);
    }

    int _Result = Natives.l_binaryWrite(filename, operation, dataPtr, nbytes);
    Marshal.FreeHGlobal(dataPtr);

    return _Result;
}
Phreak87 commented 5 years ago

thats right. make this fix in vb too

Phreak87 commented 5 years ago

but theres a error in there:

if (data.GetType().GetProperty("data") != null)
{
    var cdata = (Byte[])data; 
    // data is a class and contains a Property data ...
    dataPtr = Marshal.AllocHGlobal(cdata.Length);
    Marshal.Copy(cdata, 0, dataPtr, cdata.Length);
}

try with: LeptonicaSharp._All.l_binaryWrite("Test", "w", Pix1, Pix1.data.Length) and you will see

fdncred commented 5 years ago
public static int l_binaryWrite(
                String filename,
                String operation,
                Object data,
                uint nbytes)
{
    if (filename == null) {throw new ArgumentNullException  ("filename cannot be Nothing");}

    if (operation == null) {throw new ArgumentNullException  ("operation cannot be Nothing");}

    if (data == null) {throw new ArgumentNullException  ("data cannot be Nothing");}

    IntPtr dataPtr = IntPtr.Zero;
    if (data.GetType() == typeof(IntPtr))
        dataPtr = (IntPtr)data;
    else if (data.GetType() == typeof(Byte[]))
    {
        var cdata = (Byte[])data;
        dataPtr = Marshal.AllocHGlobal(cdata.Length);
        Marshal.Copy(cdata, 0, dataPtr, cdata.Length);
    }
    //else if (data.GetType().GetProperty("data") != null)
    //{
    //    var cdata = (Byte[])data;
    //    dataPtr = Marshal.AllocHGlobal(cdata.Length);
    //    Marshal.Copy(cdata, 0, dataPtr, cdata.Length);
    //}
    else if (data.GetType() == typeof(Pix))
    {
        var cdata = ((Pix)data).data;
        dataPtr = Marshal.AllocHGlobal(cdata.Length);
        Marshal.Copy(cdata, 0, dataPtr, cdata.Length);
    }

    int _Result = Natives.l_binaryWrite(filename, operation, dataPtr, nbytes);
    Marshal.FreeHGlobal(dataPtr);

    return _Result;
}