Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Absolute imports don't work in C++ #794

Closed
kjczarne opened this issue Aug 7, 2020 · 2 comments
Closed

Absolute imports don't work in C++ #794

kjczarne opened this issue Aug 7, 2020 · 2 comments
Milestone

Comments

@kjczarne
Copy link

kjczarne commented Aug 7, 2020

Hi, I've been trying to compile to cpp_stl and I've encountered something weird. I am not entirely sure if I am doing something wrong or whether this is a bug, so please excuse me if this seems like a newbie question ;)

I get warnings from IntelliSense that the #include "/network/ipv4_packet.h" and #include "/network/ipv6_packet.h" cannot be found:

#include "kaitai/kaitaistruct.h"
#include <stdint.h>
#include <memory>
#include "/network/ipv4_packet.h"  // <-- no folder "network"
#include "/network/ipv6_packet.h"

The compiler dumps all files into a common folder without creating a network subfolder. Compiler command used was:
ksc C:\Users\kczarnecki\Desktop\kissy\kissy\src\formats\network\pcap.ksy -t cpp_stl --cpp-namespace kaitai --cpp-standard 11

Interestingly when I manually patch every #include directive to use header files from the same folder and compile the following code:

#include <fstream>
#include <chrono>
#include <iostream>
#include "kaitaistream.h"
#include "pcap.h"

std::chrono::microseconds timing(std::string path){
    auto start = std::chrono::high_resolution_clock::now();

    std::ifstream is(path, std::ifstream::binary);
    kaitai::kstream ks(&is);
    kaitai::pcap_t data(&ks);

    auto stop = std::chrono::high_resolution_clock::now();
    auto duration = std::chrono::duration_cast<std::chrono::microseconds>(stop - start); 
    return duration;
}

int main(int argc, char *argv[]){
    for (int i = 0; i<argc; i++){
        std::cout << timing(argv[i]).count() << std::endl;
    }
}

I get a validation error, stemming from a seemingly incorrect magic number:

terminate called after throwing an instance of 'kaitai::validation_not_equal_error<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >'
  what():  /types/header/seq/0: at pos : validation failed:not equal
Aborted (core dumped)

I've set the KS_STR_ENCODING_NONE.

Parsing the very same PCAP with a Python build works perfectly. I'm using a kaitai struct compiler on Windows (commit 031c074) and the freshest cpp runtime library version.

generalmimon added a commit to kaitai-io/kaitai_struct_compiler that referenced this issue Aug 12, 2020
generalmimon added a commit that referenced this issue Aug 12, 2020
generalmimon added a commit that referenced this issue Aug 12, 2020
@generalmimon generalmimon changed the title Question to the compiler and runtime behavior on C++ Absolute import don't work in C++ Aug 14, 2020
@generalmimon
Copy link
Member

@kjczarne Thanks for reporting the bug! I found out that it has been demonstrated by test ImportsAbs (see imports_abs.h:8 before introducing the fix). The bug caused that the ImportsAbs test was failing for all C++ targets in the CI dashboard.

I fixed it in commits kaitai-io/kaitai_struct_compiler@07d71cc...f725faf and the ImportsAbs test is passing for C++ in the dashboard, so I'm closing this issue.

@generalmimon generalmimon changed the title Absolute import don't work in C++ Absolute imports don't work in C++ Aug 14, 2020
@kjczarne
Copy link
Author

Awesome, thank you! 😊

@generalmimon generalmimon added this to the v0.9 milestone Aug 25, 2020
generalmimon added a commit to kaitai-io/kaitai_struct_compiler that referenced this issue Apr 7, 2024
This code used to call the `importFile` method since its introduction is
very naive (and thus incorrect):

```scala
    curClass.meta.imports.foreach(file => lang.importFile(file))
```

It takes the full absolute/relative path found in `meta/imports` and
generates an import statement for it. So in practice, this works as
intended only if none of the `meta/imports` paths contain any slashes
(i.e. for relative .ksy imports within the same directory). KSC
currently generates all output files to the same directory, so any
non-flat import resembling a relative/absolute path is incorrect.

This became clear already in the C++ implementation of imports in KS 0.8
when it also used `importFile` -
kaitai-io/kaitai_struct#794 reported that
"absolute paths don't work", because they looked like this, which
obviously doesn't work:

```cpp
#include "/network/ipv4_packet.h"  // <-- no folder "network"
#include "/network/ipv6_packet.h
```

The solution was to stop using `importFile`.

But even if the `meta/import` path has no slashes, it's still not
guaranteed to work. Names of output files that KSC generates are derived
from `/meta/id`, but `meta/import` is a list of .ksy file paths, and
`/meta/id` and the .ksy file name are generally not required to match.
If they don't, the generated code will be broken in languages using
`importFile` (which is only Nim at the moment), for example:

main.ksy

```ksy
meta:
  id: main
  imports:
    - imported-spec
seq:
  - id: foo
    type: imported
```

imported-spec.ksy

```ksy
meta:
  id: imported
```

```console
$ kaitai-struct-compiler -- --verbose file -d compiled/nim -t nim main.ksy
parsing main.ksy...
reading main.ksy...
reading ./imported-spec.ksy...
... compiling it for nim...
.... writing main.nim
.... writing imported.nim
```

This is the beginning of the generated `main.nim`:

```nim
import kaitai_struct_nim_runtime
import options
import imported-spec

import "imported"
```

It will not compile due to invalid `import imported-spec` (since
`imported-spec.nim` doesn't exist). But note that there is already
correct `import "imported"` in place, so removing `import imported-spec`
will actually fix things.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants