Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .github/workflows/go-lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
name: Go code linters

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to invoke this exclusively for changes in the Go source code directories + go.work etc?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, fixed.

permissions:
contents: read

# Cancel workflow if there is a new change to the branch.
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}

on:
workflow_dispatch:
merge_group:
pull_request:
branches:
- main
push:
branches:
- main

jobs:
lint-go:
name: Go lint checks
runs-on: ubuntu-24.04-arm
steps:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version-file: "go.work"
- run: make lint-go
Comment thread Fixed
3 changes: 3 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
version: "2"
linters:
default: standard
6 changes: 6 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
GOLANGCI_LINT_VERSION=v2.3.1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it have to be global for the makefile? can it be just part of the go run... command?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's easier to upgrade when it's at the top, what do you think?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but it's not used anywhere else. It adds unnecessary noise for anyone NOT upgrading it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense, fixed.


install:
cargo install --locked --path . --force

Expand Down Expand Up @@ -77,6 +79,10 @@ DOCKERFILES=$(wildcard Dockerfile*)
lint-docker: $(DOCKERFILES)
docker run --rm -i hadolint/hadolint < $<

lint-go:
go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)
go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION) run --timeout 10m --concurrency 4 ./f3-sidecar ./interop-tests/src/tests/go_app

# Formats Rust, TOML and Markdown files.
fmt:
cargo fmt --all
Expand Down
3 changes: 2 additions & 1 deletion f3-sidecar/ffi_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,6 @@ func checkError(err error) {
// To avoid potential panics
// See <https://github.com/ChainSafe/forest/pull/4636#issuecomment-2306500753>
func setGoDebugEnv() {
os.Setenv("GODEBUG", "invalidptr=0,cgocheck=0")
err := os.Setenv("GODEBUG", "invalidptr=0,cgocheck=0")
checkError(err)
}
1 change: 0 additions & 1 deletion interop-tests/src/tests/go_app/.gitignore

This file was deleted.

3 changes: 2 additions & 1 deletion interop-tests/src/tests/go_app/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@ func checkError(err error) {
// To avoid potential panics
// See <https://github.com/ChainSafe/forest/pull/4636#issuecomment-2306500753>
func setGoDebugEnv() {
os.Setenv("GODEBUG", "invalidptr=0,cgocheck=0")
err := os.Setenv("GODEBUG", "invalidptr=0,cgocheck=0")
checkError(err)
}
237 changes: 237 additions & 0 deletions interop-tests/src/tests/go_app/gen.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,237 @@
package main

/*
// Generated by rust2go. Please DO NOT edit this C part manually.

#include <stdarg.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdlib.h>

typedef struct ListRef {
const void *ptr;
uintptr_t len;
} ListRef;

typedef struct StringRef {
const uint8_t *ptr;
uintptr_t len;
} StringRef;
*/
import "C"
import (
"runtime"
"unsafe"

"github.com/ihciah/rust2go/asmcall"
)
Comment on lines +1 to +

@coderabbitai coderabbitai Bot Aug 7, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Consider excluding generated files from version control and linting

This file is marked as generated by rust2go with an explicit "DO NOT edit" warning. Generated files should typically be:

  1. Excluded from version control (regenerated as needed)
  2. Excluded from linting to avoid noise from auto-generated code

The AI summary indicates a .gitignore entry for gen.go was removed. Consider keeping generated files out of version control and adding this path to .golangci.yml exclusions.


🏁 Script executed:

#!/bin/bash
# Check if other generated files are tracked and if linting exclusions are configured

# Check for other generated files in the repository
echo "=== Searching for other generated files ==="
rg -l "Generated by|DO NOT edit|Code generated" --type go

# Check if gen.go is excluded from linting in .golangci.yml
echo -e "\n=== Checking .golangci.yml for exclusions ==="
if [ -f ".golangci.yml" ]; then
    cat .golangci.yml | grep -A 10 -B 2 "skip-files\|skip-dirs\|exclude"
fi

# Check if there's a generation script or Makefile target
echo -e "\n=== Checking for generation commands ==="
if [ -f "Makefile" ]; then
    grep -n "gen\.go\|rust2go" Makefile
fi

Length of output: 578


Exclude generated Go files from version control and linting

We discovered two tracked generated files without any lint-exclusion rules:

  • interop-tests/src/tests/go_app/gen.go
  • f3-sidecar/ffi_gen.go

And .golangci.yml contains no skip-files, skip-dirs, or exclude entries for them.

Action items:

  • Add these paths (or a regex) to .gitignore so they aren’t accidentally committed in the future.
  • Configure GolangCI to skip linting on all *_gen.go files.

Suggested diff:

# .gitignore
+# Ignore Rust2Go-generated Go code
+**/*_gen.go

# .golangci.yml
 run:
+  skip-files:
+    - '.*_gen\.go$'

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In interop-tests/src/tests/go_app/gen.go lines 1 to 27, the file is a generated
Go file but is currently tracked in version control and not excluded from
linting. To fix this, add the path interop-tests/src/tests/go_app/gen.go or a
suitable pattern matching generated files to the .gitignore file to prevent
accidental commits. Additionally, update the .golangci.yml configuration to
include a skip-files or exclude rule that matches all *_gen.go files, ensuring
these generated files are excluded from linting.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hanabi1224 I think this comment makes sense.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LesnyRumcajs Fixed. (The suggested change does not match the config syntax though)


var GoKadNodeImpl GoKadNode

type GoKadNode interface {
run()
connect(multiaddr *string)
get_n_connected() uint
}

//export CGoKadNode_run
func CGoKadNode_run() {
GoKadNodeImpl.run()
}

//export CGoKadNode_connect
func CGoKadNode_connect(multiaddr C.StringRef) {
_new_multiaddr := newString(multiaddr)
GoKadNodeImpl.connect(&_new_multiaddr)
}

//export CGoKadNode_get_n_connected
func CGoKadNode_get_n_connected(slot *C.void, cb *C.void) {
resp := GoKadNodeImpl.get_n_connected()
resp_ref, buffer := cvt_ref(cntC_uintptr_t, refC_uintptr_t)(&resp)
asmcall.CallFuncG0P2(unsafe.Pointer(cb), unsafe.Pointer(&resp_ref), unsafe.Pointer(slot))
runtime.KeepAlive(resp_ref)
runtime.KeepAlive(resp)
runtime.KeepAlive(buffer)
}

var GoBitswapNodeImpl GoBitswapNode

type GoBitswapNode interface {
run()
connect(multiaddr *string)
get_block(cid *string) bool
}

//export CGoBitswapNode_run
func CGoBitswapNode_run() {
GoBitswapNodeImpl.run()
}

//export CGoBitswapNode_connect
func CGoBitswapNode_connect(multiaddr C.StringRef) {
_new_multiaddr := newString(multiaddr)
GoBitswapNodeImpl.connect(&_new_multiaddr)
}

//export CGoBitswapNode_get_block
func CGoBitswapNode_get_block(cid C.StringRef, slot *C.void, cb *C.void) {
_new_cid := newString(cid)
resp := GoBitswapNodeImpl.get_block(&_new_cid)
resp_ref, buffer := cvt_ref(cntC_bool, refC_bool)(&resp)
asmcall.CallFuncG0P2(unsafe.Pointer(cb), unsafe.Pointer(&resp_ref), unsafe.Pointer(slot))
runtime.KeepAlive(resp_ref)
runtime.KeepAlive(resp)
runtime.KeepAlive(buffer)
}

func newString(s_ref C.StringRef) string {
return unsafe.String((*byte)(unsafe.Pointer(s_ref.ptr)), s_ref.len)
}
func refString(s *string, _ *[]byte) C.StringRef {
return C.StringRef{
ptr: (*C.uint8_t)(unsafe.StringData(*s)),
len: C.uintptr_t(len(*s)),
}
}

func ownString(s_ref C.StringRef) string {
return string(unsafe.Slice((*byte)(unsafe.Pointer(s_ref.ptr)), int(s_ref.len)))
}
Comment on lines +88 to +

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Memory safety concern with string conversion functions

The newString function (line 89) creates a Go string that aliases the C memory without copying, while ownString (line 99) creates a proper copy. Using newString in the exported functions (lines 44, 73, 79) could lead to use-after-free if the C side deallocates the memory while Go still holds references.

Consider using ownString instead of newString in the exported functions for safety, unless you're certain the C memory remains valid for the entire duration of the Go function calls.

🤖 Prompt for AI Agents
In interop-tests/src/tests/go_app/gen.go around lines 88 to 100, the newString
function returns a Go string referencing C memory without copying, risking
use-after-free if C deallocates it. To fix this, replace calls to newString in
the exported functions at lines 44, 73, and 79 with ownString, which safely
copies the C memory into a Go string, ensuring memory safety.

func cntString(_ *string, _ *uint) [0]C.StringRef { return [0]C.StringRef{} }
func new_list_mapper[T1, T2 any](f func(T1) T2) func(C.ListRef) []T2 {
return func(x C.ListRef) []T2 {
input := unsafe.Slice((*T1)(unsafe.Pointer(x.ptr)), x.len)
output := make([]T2, len(input))
for i, v := range input {
output[i] = f(v)
}
return output
}
}
func new_list_mapper_primitive[T1, T2 any](_ func(T1) T2) func(C.ListRef) []T2 {
return func(x C.ListRef) []T2 {
return unsafe.Slice((*T2)(unsafe.Pointer(x.ptr)), x.len)
}
}

// only handle non-primitive type T
func cnt_list_mapper[T, R any](f func(s *T, cnt *uint) [0]R) func(s *[]T, cnt *uint) [0]C.ListRef {
return func(s *[]T, cnt *uint) [0]C.ListRef {
for _, v := range *s {
f(&v, cnt)
}
*cnt += uint(len(*s)) * size_of[R]()
return [0]C.ListRef{}
}
}

// only handle primitive type T
func cnt_list_mapper_primitive[T, R any](_ func(s *T, cnt *uint) [0]R) func(s *[]T, cnt *uint) [0]C.ListRef {
return func(s *[]T, cnt *uint) [0]C.ListRef { return [0]C.ListRef{} }
}

// only handle non-primitive type T
func ref_list_mapper[T, R any](f func(s *T, buffer *[]byte) R) func(s *[]T, buffer *[]byte) C.ListRef {
return func(s *[]T, buffer *[]byte) C.ListRef {
if len(*buffer) == 0 {
return C.ListRef{
ptr: unsafe.Pointer(nil),
len: C.uintptr_t(len(*s)),
}
}
ret := C.ListRef{
ptr: unsafe.Pointer(&(*buffer)[0]),
len: C.uintptr_t(len(*s)),
}
children_bytes := int(size_of[R]()) * len(*s)
children := (*buffer)[:children_bytes]
*buffer = (*buffer)[children_bytes:]
for _, v := range *s {
child := f(&v, buffer)
len := unsafe.Sizeof(child)
copy(children, unsafe.Slice((*byte)(unsafe.Pointer(&child)), len))
children = children[len:]
}
return ret
}
}

// only handle primitive type T
func ref_list_mapper_primitive[T, R any](_ func(s *T, buffer *[]byte) R) func(s *[]T, buffer *[]byte) C.ListRef {
return func(s *[]T, buffer *[]byte) C.ListRef {
if len(*s) == 0 {
return C.ListRef{
ptr: unsafe.Pointer(nil),
len: C.uintptr_t(0),
}
}
return C.ListRef{
ptr: unsafe.Pointer(&(*s)[0]),
len: C.uintptr_t(len(*s)),
}
}
}
func size_of[T any]() uint {
var t T
return uint(unsafe.Sizeof(t))
}
func cvt_ref[R, CR any](cnt_f func(s *R, cnt *uint) [0]CR, ref_f func(p *R, buffer *[]byte) CR) func(p *R) (CR, []byte) {
return func(p *R) (CR, []byte) {
var cnt uint
cnt_f(p, &cnt)
buffer := make([]byte, cnt)
return ref_f(p, &buffer), buffer
}
}
func cvt_ref_cap[R, CR any](cnt_f func(s *R, cnt *uint) [0]CR, ref_f func(p *R, buffer *[]byte) CR, add_cap uint) func(p *R) (CR, []byte) {
return func(p *R) (CR, []byte) {
var cnt uint
cnt_f(p, &cnt)
buffer := make([]byte, cnt, cnt+add_cap)
return ref_f(p, &buffer), buffer
}
}

func newC_uint8_t(n C.uint8_t) uint8 { return uint8(n) }
func newC_uint16_t(n C.uint16_t) uint16 { return uint16(n) }
func newC_uint32_t(n C.uint32_t) uint32 { return uint32(n) }
func newC_uint64_t(n C.uint64_t) uint64 { return uint64(n) }
func newC_int8_t(n C.int8_t) int8 { return int8(n) }
func newC_int16_t(n C.int16_t) int16 { return int16(n) }
func newC_int32_t(n C.int32_t) int32 { return int32(n) }
func newC_int64_t(n C.int64_t) int64 { return int64(n) }
func newC_bool(n C.bool) bool { return bool(n) }
func newC_uintptr_t(n C.uintptr_t) uint { return uint(n) }
func newC_intptr_t(n C.intptr_t) int { return int(n) }
func newC_float(n C.float) float32 { return float32(n) }
func newC_double(n C.double) float64 { return float64(n) }

func cntC_uint8_t(_ *uint8, _ *uint) [0]C.uint8_t { return [0]C.uint8_t{} }
func cntC_uint16_t(_ *uint16, _ *uint) [0]C.uint16_t { return [0]C.uint16_t{} }
func cntC_uint32_t(_ *uint32, _ *uint) [0]C.uint32_t { return [0]C.uint32_t{} }
func cntC_uint64_t(_ *uint64, _ *uint) [0]C.uint64_t { return [0]C.uint64_t{} }
func cntC_int8_t(_ *int8, _ *uint) [0]C.int8_t { return [0]C.int8_t{} }
func cntC_int16_t(_ *int16, _ *uint) [0]C.int16_t { return [0]C.int16_t{} }
func cntC_int32_t(_ *int32, _ *uint) [0]C.int32_t { return [0]C.int32_t{} }
func cntC_int64_t(_ *int64, _ *uint) [0]C.int64_t { return [0]C.int64_t{} }
func cntC_bool(_ *bool, _ *uint) [0]C.bool { return [0]C.bool{} }
func cntC_uintptr_t(_ *uint, _ *uint) [0]C.uintptr_t { return [0]C.uintptr_t{} }
func cntC_intptr_t(_ *int, _ *uint) [0]C.intptr_t { return [0]C.intptr_t{} }
func cntC_float(_ *float32, _ *uint) [0]C.float { return [0]C.float{} }
func cntC_double(_ *float64, _ *uint) [0]C.double { return [0]C.double{} }

func refC_uint8_t(p *uint8, _ *[]byte) C.uint8_t { return C.uint8_t(*p) }
func refC_uint16_t(p *uint16, _ *[]byte) C.uint16_t { return C.uint16_t(*p) }
func refC_uint32_t(p *uint32, _ *[]byte) C.uint32_t { return C.uint32_t(*p) }
func refC_uint64_t(p *uint64, _ *[]byte) C.uint64_t { return C.uint64_t(*p) }
func refC_int8_t(p *int8, _ *[]byte) C.int8_t { return C.int8_t(*p) }
func refC_int16_t(p *int16, _ *[]byte) C.int16_t { return C.int16_t(*p) }
func refC_int32_t(p *int32, _ *[]byte) C.int32_t { return C.int32_t(*p) }
func refC_int64_t(p *int64, _ *[]byte) C.int64_t { return C.int64_t(*p) }
func refC_bool(p *bool, _ *[]byte) C.bool { return C.bool(*p) }
func refC_uintptr_t(p *uint, _ *[]byte) C.uintptr_t { return C.uintptr_t(*p) }
func refC_intptr_t(p *int, _ *[]byte) C.intptr_t { return C.intptr_t(*p) }
func refC_float(p *float32, _ *[]byte) C.float { return C.float(*p) }
func refC_double(p *float64, _ *[]byte) C.double { return C.double(*p) }
func main() {}
Loading