incorrect code generated for `char x[8][255]`

[ghorn]

I think this issue I posted in c2hsc belongs here instead:

https://github.com/jwiegley/c2hsc/issues/25

[ghorn]

(copied here, sorry for the noise:)

I have a struct like:

typedef struct {
    char listOfNames[8][255];
} MyCoolStruct;

I would expect that field to translate to type [Ptr CChar], which correctly happens, woohoo!

The problem is that I expect a list of length 8, where each Ptr CChar points to a character array with length 255. Instead I get a list of length 255, where the pointers are all invalid.

In the final haskell code, the deserialization looks like

instance Storable C'MyCoolStruct where
  peek p = do
    v0 <- peekArray (2040 `div` (sizeOf (undefined :: Ptr CChar)) (plusPtr p 0)
    return (C'MyCoolStruct v0)

which translates to

instance Storable C'MyCoolStruct where
  peek p = do
    v0 <- peekArray 255 p :: IO [Ptr CChar]
    return (C'MyCoolStruct v0)

This gives corrupt output. The pointers aren't valid pointers, they are strings cast to pointers. I think the correct implementation would be:

instance Storable C'MyCoolStruct where
  peek p = do
    let v0 = [plusPtr (plusPtr p 0) (255 * k) | k <- [0..8]]
    return (C'MyCoolStruct v0)

I looked through the source code but didn't make much progress. I'll be happy to try again if you can give me a few pointers to relevant sections.

[jwiegley]

Thanks, Greg! I'm mentoring at a Hackathon next week, and this would be a great bug to get someone interested in. I'll let you know how it goes.

[jwiegley]

This looks like it's happening at https://github.com/jwiegley/bindings-dsl/blob/master/bindings.dsl.h#L313, and that we need to take the size of any sub-array into account, rather than always using the size of Ptr CChar (which in this case is likely 8 bytes, and coincidentally the same size as your outer array bound).

[ghorn]

It seems to me that the major size is not available, only the total number of elements in the multi array. Is that right? So C2hsc.hs itself needs the patch?

[jwiegley]

That sounds right.

[ghorn]

In the meanwhile, my workaround is:

typedef struct {
  char name[255];
} MyString;

typedef struct {
  MyString stringArray[8];
} MyStruct;

[ghorn]

Do you think you can find time to take a look at this in the next few weeks? I understand if you're wrapped up in other things.

I would greatly appreciate even a hacky unfinished patch which I would try to polish. I tried to fix this myself but I find language-c very opaque.

[jwiegley]

@ghorn What is the context in which you're using this?

[ghorn]

At work we have a large C/C++ codebase. I have a script which extracts all the relevant typedef structs from this codebase and writes them into one header file. I use c2hsc / hsc2hs to turn this header file into a haskell module where every data type is an instance of Storable. This lets me deserialize log files and analyze them in haskell.

[jwiegley]

Ah, so this is for getting real work done. I'll put it on my todo list for the week and see if I can't get to it for you. I almost got somebody at our last local Hackathon to look at this particular bug, but then it fell through. It shouldn't be hard.

[ghorn]

you have my gratitude

[jwiegley]

@ghorn It should be fixed now in the c2hsc master branch, can you please confirm?

[ghorn]

Thank you! There may be a few problems. See my comments here https://github.com/jwiegley/c2hsc/commit/98930351c288b47aa2beb28bc35612cb351937fe

[ghorn]

After the fix, the behavior is to turn

typedef struct {
    char listOfNames[8][255];
} MyCoolStruct;

into:

data C'MyCoolStruct = C'MyCoolStruct{
  c'MyCoolStruct'listOfNames :: [CChar]
} deriving (Eq,Show)

Is that the intended behavior?

I would have expected

data C'MyCoolStruct = C'MyCoolStruct{
  c'MyCoolStruct'listOfNames :: [[CChar]]
} deriving (Eq,Show)

The problem is now that I can't easily turn this into [String] because I don't have the information to divide this into 8 groups before I turn [CChar] into String.

[jwiegley]

@ghorn bindings-DSL is a bit coarse in its translation of multi-dimensional arrays. The new mapping matches the Storable instance that it generates. We'll have to explore better representation of multiple dimensions as a new issue, since that should be fixed as well. Even if we were to emit the translation as [[CChar]] from c2hsc, the Storable instance would still be wrong.

[jwiegley]

It may even be that we need a new argument to #array_field to support this, since knowing the type doesn't tell us enough about the array's size.

[ghorn]

I don't completely follow this. Are you saying that bindings-dsl is generating the Storable instance, and there is no interface which lets c2hsc do what I want? So there is no c2hsc solution which doesn't involve changing bindings-dsl? Is this correct?

[ghorn]

BTW did you see the other two comments? The module name problem https://github.com/jwiegley/c2hsc/commit/98930351c288b47aa2beb28bc35612cb351937fe#commitcomment-13723380 and the top level array typedef problem https://github.com/jwiegley/c2hsc/commit/98930351c288b47aa2beb28bc35612cb351937fe#commitcomment-13723516 ?

[jwiegley]

@ghorn Yes, I did see them, thanks! I'll create separate issues for these three things.