Hi,

I'm trying to create bindings to the GraphcicsMagick C library which has the following struct defined:

    #define MaxTextExtent 2053

    typedef struct _Image {
      /* other members skipped */
      char filename[MaxTextExtent];
    } Image;


In an `extern (C)` block I've "converted" the struct to D:

    enum MaxTextExtent = 2053;

    struct Image
    {
      /* other members skipped */
      char[MaxTextExtent] filename;
    }


The problem is trying to set the `filename` member since I can't directly use a D string.  I've seen a post which is a similar issue[0], but adapting the code provides an error:
    static assert: "Cannot put a string into a char[2053]"

I've also tried using `new char[MaxTextExtent]` and looping through the string to copy the characters over.  This does compile, however, when trying to write the image to disk the provided filename is often a "stripped" version of when you want (e.g. "image.jpg" becomes "g").

[0]: https://forum.dlang.org/thread/lshnzqbfkrhfkliapicm@forum.dlang.org

import std;
void main()
{
    int[] a = [1, 2, 3, 4, 5];
    int[3] b;

    b[0 .. 3] = a[1 .. 4];
    b.writeln;
}

Same principle, just remember to null terminate after slicing your dynamic array and assigning it to your static array.

On Sunday, 3 January 2021 at 09:28:55 UTC, rikki cattermole wrote:
> import std;
> void main()
> {
>     int[] a = [1, 2, 3, 4, 5];
>     int[3] b;
>
>     b[0 .. 3] = a[1 .. 4];
>     b.writeln;
> }
>
> Same principle, just remember to null terminate after slicing your dynamic array and assigning it to your static array.

Thanks for the suggestion, however, it yields the same results as my "create new array and loop through string" attempt.  That is to say, an incomplete filename is used when writing (saving) the image to disk.

If it help, here is some of the actual code (using what I've understood from your sample):

    int main(string[] args)
    {
      ulong len;

      Image* image;
      /* a separate structure, which also has a char[MaxTextExtent] filename */
      ImageInfo* imageInfo;
      /* another GraphicsMagick struct */
      ExceptionInfo exception;

      InitializeMaigck(null);
      /* creates a ImageInfo with defaults */
      imageInfo = CloneImageInfo(null);

      /* actual code has a check for args.length */
      len = args[1].length;

      /* set the input filename */
      imageInfo.filename[0 .. len] = args[1];
      imageInfo.filename[len] = '\0';

      /* read image from input file */
      image = ReadImage(imageInfo, &exception);

      /* set the output filename */
      len = args[2].length;
      image.filename[0 .. len] = args[2];
      image.filename[len] = '\0';

      /* write (save) image to disk */
      WriteImage(imageInfo, image);

      /* memory cleanup skipped */

      return 0;
    }

The full code is really just a port of the `convert.c` example shown on the API page[0],

[0]: http://www.graphicsmagick.org/api/api.html

Your definition of Image is probably wrong.

You may have missed a pointer (8 bytes).

On Sunday, 3 January 2021 at 11:16:25 UTC, rikki cattermole wrote:
> Your definition of Image is probably wrong.
>
> You may have missed a pointer (8 bytes).

I'm pretty sure it's correct?

Here's the D version:
  https://repo.or.cz/magickd.git/blob/e5d06e939:/source/magickd/core/c/image.d#l751

Here's the C version:
  http://hg.code.sf.net/p/graphicsmagick/code/file/a622095da492/magick/image.h#l683

On Sunday, 3 January 2021 at 08:43:34 UTC, bdh wrote:
> Hi,
>
> I'm trying to create bindings to the GraphcicsMagick C library which has the following struct defined:
>
> [...]

Do you mean fill .filename member from a D string? something like this?

>import std.stdio;
>
> [...]

>    struct Image
>    {
>      /* other members skipped */
>      char[MaxTextExtent] filename;
>    }
>}
>
> [...]
>import std.stdio;
>
>extern(C)
>{
> 
>    enum MaxTextExtent = 2053;

>    struct Image
>    {
>      /* other members skipped */
>      char[MaxTextExtent] filename;
>    }
>}
>
>void main()
>{
>    import std.string : toStringz;
>    import core.stdc.string : memcpy, strlen;
>
>    auto i = Image();
>    auto dstr = "hello world!!!"; // D string
>    auto cs = toStringz(dstr); // now it's C-null-terminatted string

>    // since .filename isn't a pointer but an array, I think
>    // you have to use memcpy() here. = operator wouldn't work properly.
>    memcpy(&i.filename[0], &cs[0], strlen(cs)+1);
>
>    // casting away arrayness to make it a pointer (that a C's array is after all)
>    printf("str = [%s]\n", &i.filename[0]);
>}
>
> [...]

note that even if .filename was a pointer, in order to the C converted stirng don't turn into garbage in a GC cycle, you would have to either keep reference to dstr around or malloc() and memcpy() it. So you need to convert from C to D: you can use fromStringz() on D string then to!string to convert to a D string, like this:

> import std.conv : to;
> import std.string : fromStrinz;
> string s = to!string(cstr.fromStringz);

On 1/3/21 9:17 PM, bdh wrote:
> On Sunday, 3 January 2021 at 11:16:25 UTC, rikki cattermole wrote:
>> Your definition of Image is probably wrong.
>>
>> You may have missed a pointer (8 bytes).
> 
> I'm pretty sure it's correct?
> 
> Here's the D version:
> https://repo.or.cz/magickd.git/blob/e5d06e939:/source/magickd/core/c/image.d#l751 
> 
> 
> Here's the C version:
> http://hg.code.sf.net/p/graphicsmagick/code/file/a622095da492/magick/image.h#l683 
> 
> 

How to test:

in D-land:

import magickd.core.c.image : Image;

pragma(msg, Image.filename.offsetof);

In C-land

#include <stdio.h>
#include <magick/image.h>

int main()
{
   Image *ptr = NULL;
   printf("%ld", &(ptr->filename)); // may need a different specifier
}

If those are not the same, then your struct is off.

-Steve

On Monday, 4 January 2021 at 02:17:33 UTC, bdh wrote:
> I'm pretty sure it's correct?
>
> Here's the D version:
>   https://repo.or.cz/magickd.git/blob/e5d06e939:/source/magickd/core/c/image.d#l751
>
> Here's the C version:
>   http://hg.code.sf.net/p/graphicsmagick/code/file/a622095da492/magick/image.h#l683


> /* XXX: haven't done private members yet, wonder if they're needed to
>  * complete this */

Leaving out the private members of a struct changes its size, and sometimes its alignment. This will definitely break things if you ever allocate an instance yourself (including via the stack).

If you only EVER access that type via pointers to instances allocated by an API which internally uses the complete, correct definition, and ALL of the private members are at the end of the struct, it should work - but those are two big "if"s, and too easy to unthinkingly violate later by accident. So, it is best to ensure that the struct definition matches precisely on both the D and C sides of the API, or at least enough to ensure that the size and alignment are the same.

On Monday, 4 January 2021 at 16:35:23 UTC, Jack wrote:
> Do you mean fill .filename member from a D string? something like this?
>
> [...]
>
>     // since .filename isn't a pointer but an array, I think
>     // you have to use memcpy() here. = operator wouldn't work properly.
>     memcpy(&i.filename[0], &cs[0], strlen(cs)+1);

I'm pretty sure I tried using = but it resulted in a type error (or array length mismatch).
Also tried memcpy which compiled, though due to the incorrect struct definition it appeared to not work.

>     // casting away arrayness to make it a pointer (that a C's array is after all)
>     printf("str = [%s]\n", &i.filename[0]);

I'll keep this and ...

> note that even if .filename was a pointer, in order to the C converted stirng don't turn into garbage in a GC cycle, you would have to either keep reference to dstr around or malloc() and memcpy()

... this in mind, thanks.

On Monday, 4 January 2021 at 17:24:27 UTC, Steven Schveighoffer wrote:
> On 1/3/21 9:17 PM, bdh wrote:
>> On Sunday, 3 January 2021 at 11:16:25 UTC, rikki cattermole wrote:
>>> Your definition of Image is probably wrong.
>>>
>>> You may have missed a pointer (8 bytes).
>> 
>> [...]
>> 
>
> How to test:
>
> in D-land:
>
> import magickd.core.c.image : Image;
>
> pragma(msg, Image.filename.offsetof);
>
> In C-land
>
> #include <stdio.h>
> #include <magick/image.h>
>
> int main()
> {
>    Image *ptr = NULL;
>    printf("%ld", &(ptr->filename)); // may need a different specifier
> }
>
> If those are not the same, then your struct is off.
>
> -Steve

Thanks for this, I wasn't aware of the .offsetof property (I see it's mentioned in the Struct and Unions section of the language reference).

As rikki guessed, I had missed something (offset was 480 instead of 488).  The issue wasn't with the Image struct itself, but with the TimerInfo struct (used for the `timer` member of Image) missing a "private" unsigned long.

Thanks again.