Skip to content

Arrays in a C struct #33

Description

@dmbates

I noticed in reading the code that the Julia struct

struct ReadStatVariable
    readstat_types_t::Cint
    index::Cint
    name::Ptr{UInt8}
    format::Ptr{UInt8}
    label::Ptr{UInt8}
    label_set::Ptr{Void}
    offset::Int64
    width::Csize_t
    user_width::Csize_t
    missingness::ReadStatMissingness 
end

does not align with the C declaration in readstat,h

typedef struct readstat_variable_s {
    readstat_type_t         type;
    int                     index;
    char                    name[256];
    char                    format[256];
    char                    label[1024];
    readstat_label_set_t   *label_set;
    off_t                   offset;
    size_t                  storage_width;
    size_t                  user_width;
    readstat_missingness_t  missingness;
    readstat_measure_t      measure;
    readstat_alignment_t    alignment;
    int                     display_width;
} readstat_variable_t;

The problem is that the fixed-size char arrays, like name do not correspond to Ptr{UInt8} or Cstring in Julia. If you want the structs to align in memory you need to insert a Julia struct that has the correct width. The Clang package at one time did this by creating a Julia struct with 256 UInt8 fields and inserting that as the name field. However, that approach is rather tedious.

In this case there is no need to define the Julia struct because the memory is allocated in the C code and passed by reference. I can prepare a PR if you wish.

Activity

  1. davidanthoff commented on Jan 13, 2018

    @davidanthoff
    Member

    Yes, that would be great!

    Just one warning: we use a quite old version of the readstat lib. I've started work to get us up to speed with the latest version by using BinaryBuilder and compiling a new version, but that is not done yet. I'm not sure whether there are any changes to these types upstream, but just something to keep in mind.

  2. andreasnoack commented on Jan 16, 2018

    @andreasnoack
    Contributor

    I looked at the main repo and it seems that they have changed that struct quite a few times so if we really want to match it in Julia, we'd have to be careful with checking the version of the linked library. However, there are quite a few getter functions defined so it is safer and simpler to just rely on them instead of trying to match the struct in Julia.

  3. davidanthoff commented on Jan 16, 2018

    @davidanthoff
    Member

    So that maybe suggests that we don't even need to define these structs at all, but can just pass a pointer around and then use the getter functions to access things, right? That seems an all around safer approach.

  4. dmbates commented on Jan 16, 2018

    @dmbates
    ContributorAuthor

    @davidanthoff Yes. I think it will be safer overall to use the getter functions and not try to emulate the C structs. I am working on a PR.

  5. dmbates commented on Jan 16, 2018

    @dmbates
    ContributorAuthor

    I was wrong about passing readstat_value_t as an opaque pointer. It is passed by value in the C code so there must be a Julia struct like ReadStatValue, which must have the right size, as you have ensured.

  6. davidanthoff commented on Jan 16, 2018

    @davidanthoff
    Member

    Well, I didn't write any of this code ;) This julia package was original created by Evan Miller, who is the creator for the ReadStat C library. He is no longer interested in maintaining the julia package, so he transferred the repo over to me because I had helped out getting it to run on Windows a while back. So really he deserves the credit for any of this here.

  7. dmbates commented on Jan 16, 2018

    @dmbates
    ContributorAuthor

    @davidanthoff Would you be willing to transfer ownership to the JuliaData group?

  8. davidanthoff commented on Jan 16, 2018

    @davidanthoff
    Member

    I'm happy to keep maintaining it. I like the model where someone is on the hook if things don't work, and for now I'm happy to be that person ;) If I run out of time to do that, we could move it over.

  9. added theissue type on Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions