fix: correct generated PipeWire adapter output
- commit
- 43acd17876702ecd9df8df35cac8c2ce9cdad171
- author
- brice · · 2026-04-10 12:44 UTC
- parent
- 9d99e50
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +56 -31
M cmd/pipewiregen/internal/emitter/emitter.go +40 -31
| @@ -253,6 +253,31 @@ func parseSignature(sig string) (params, results string) { | |||
| 253 | 253 | return params, results | |
| 254 | 254 | } | |
| 255 | 255 | ||
| 256 | + | // extractParamNames parses a typed parameter list like "(argc *int32, argv ***byte)" | |
| 257 | + | // and returns a call-site argument list like "(argc, argv)". | |
| 258 | + | // It strips all type information, keeping only the parameter names. | |
| 259 | + | func extractParamNames(params string) string { | |
| 260 | + | inner := strings.Trim(params, "()") | |
| 261 | + | if inner == "" { | |
| 262 | + | return "()" | |
| 263 | + | } | |
| 264 | + | ||
| 265 | + | parts := strings.Split(inner, ",") | |
| 266 | + | names := make([]string, 0, len(parts)) | |
| 267 | + | for _, part := range parts { | |
| 268 | + | part = strings.TrimSpace(part) | |
| 269 | + | // Each part is "name type" or "name type1/type2" — the first space separates name from type. | |
| 270 | + | idx := strings.IndexByte(part, ' ') | |
| 271 | + | if idx < 0 { | |
| 272 | + | // No space means just a name with no type (shouldn't happen in valid Go, but handle it) | |
| 273 | + | names = append(names, part) | |
| 274 | + | } else { | |
| 275 | + | names = append(names, part[:idx]) | |
| 276 | + | } | |
| 277 | + | } | |
| 278 | + | return "(" + strings.Join(names, ", ") + ")" | |
| 279 | + | } | |
| 280 | + | ||
| 256 | 281 | // filterSymbolsByGroup returns symbols that belong to the given group. | |
| 257 | 282 | func filterSymbolsByGroup(symbols []model.Symbol, groupName string) []model.Symbol { | |
| 258 | 283 | var result []model.Symbol | |
| @@ -295,10 +320,11 @@ func toCamelCase(s string) string { | |||
| 295 | 320 | // renderAdapters generates one thin adapter struct per group with forwarding methods. | |
| 296 | 321 | func renderAdapters(m *model.Model) ([]byte, error) { | |
| 297 | 322 | type AdapterMethod struct { | |
| 298 | - | CName string | |
| 299 | - | GoName string | |
| 300 | - | Params string | |
| 301 | - | Results string | |
| 323 | + | CName string | |
| 324 | + | GoName string | |
| 325 | + | Params string | |
| 326 | + | CallArgs string | |
| 327 | + | Results string | |
| 302 | 328 | } | |
| 303 | 329 | ||
| 304 | 330 | type AdapterGroup struct { | |
| @@ -315,10 +341,11 @@ func renderAdapters(m *model.Model) ([]byte, error) { | |||
| 315 | 341 | for _, sym := range groupSyms { | |
| 316 | 342 | params, results := parseSignature(sym.Signature) | |
| 317 | 343 | methods = append(methods, AdapterMethod{ | |
| 318 | - | CName: sym.Name, | |
| 319 | - | GoName: toGoName(sym.Name), | |
| 320 | - | Params: params, | |
| 321 | - | Results: results, | |
| 344 | + | CName: sym.Name, | |
| 345 | + | GoName: toGoName(sym.Name), | |
| 346 | + | Params: params, | |
| 347 | + | CallArgs: extractParamNames(params), | |
| 348 | + | Results: results, | |
| 322 | 349 | }) | |
| 323 | 350 | if strings.Contains(sym.Signature, "unsafe.Pointer") { | |
| 324 | 351 | needsUnsafe = true | |
| @@ -348,26 +375,10 @@ func renderAdapters(m *model.Model) ([]byte, error) { | |||
| 348 | 375 | ||
| 349 | 376 | // renderComposite generates the composite CAPI interface that embeds all group interfaces. | |
| 350 | 377 | func renderComposite(m *model.Model) ([]byte, error) { | |
| 351 | - | // Determine if any group uses unsafe.Pointer | |
| 352 | - | needsUnsafe := false | |
| 353 | - | for _, group := range m.Groups { | |
| 354 | - | for _, sym := range filterSymbolsByGroup(m.Symbols, group.Name) { | |
| 355 | - | if strings.Contains(sym.Signature, "unsafe.Pointer") { | |
| 356 | - | needsUnsafe = true | |
| 357 | - | break | |
| 358 | - | } | |
| 359 | - | } | |
| 360 | - | if needsUnsafe { | |
| 361 | - | break | |
| 362 | - | } | |
| 363 | - | } | |
| 364 | - | ||
| 365 | 378 | data := struct { | |
| 366 | - | Groups []model.Group | |
| 367 | - | NeedsUnsafe bool | |
| 379 | + | Groups []model.Group | |
| 368 | 380 | }{ | |
| 369 | - | Groups: m.Groups, | |
| 370 | - | NeedsUnsafe: needsUnsafe, | |
| 381 | + | Groups: m.Groups, | |
| 371 | 382 | } | |
| 372 | 383 | ||
| 373 | 384 | var buf bytes.Buffer | |
| @@ -450,9 +461,9 @@ type {{$group.AdapterName}} struct{} | |||
| 450 | 461 | {{range $group.Methods}} | |
| 451 | 462 | func (a *{{$group.AdapterName}}) {{.GoName}}{{.Params}}{{if .Results}} {{.Results}}{{end}} { | |
| 452 | 463 | {{- if .Results}} | |
| 453 | - | return {{.CName}}{{.Params}} | |
| 464 | + | return {{.CName}}{{.CallArgs}} | |
| 454 | 465 | {{- else}} | |
| 455 | - | {{.CName}}{{.Params}} | |
| 466 | + | {{.CName}}{{.CallArgs}} | |
| 456 | 467 | {{- end}} | |
| 457 | 468 | } | |
| 458 | 469 | {{end}} | |
| @@ -462,9 +473,7 @@ func (a *{{$group.AdapterName}}) {{.GoName}}{{.Params}}{{if .Results}} {{.Result | |||
| 462 | 473 | const compositeTemplate = `// Code generated by pipewiregen. DO NOT EDIT. | |
| 463 | 474 | ||
| 464 | 475 | package out | |
| 465 | - | {{if .NeedsUnsafe}} | |
| 466 | - | import "unsafe" | |
| 467 | - | {{end}} | |
| 476 | + | ||
| 468 | 477 | // CAPI is the composite interface that embeds all generated group interfaces. | |
| 469 | 478 | type CAPI interface { | |
| 470 | 479 | {{- range .Groups}} | |
| @@ -253,6 +253,31 @@ func parseSignature(sig string) (params, results string) { | |||||
| 253 | return params, results | 253 | return params, results | ||
| 254 | } | 254 | } | ||
| 255 | 255 | ||||
| 256 | + | // extractParamNames parses a typed parameter list like "(argc *int32, argv ***byte)" | |||
| 257 | + | // and returns a call-site argument list like "(argc, argv)". | |||
| 258 | + | // It strips all type information, keeping only the parameter names. | |||
| 259 | + | func extractParamNames(params string) string { | |||
| 260 | + | inner := strings.Trim(params, "()") | |||
| 261 | + | if inner == "" { | |||
| 262 | + | return "()" | |||
| 263 | + | } | |||
| 264 | + | ||||
| 265 | + | parts := strings.Split(inner, ",") | |||
| 266 | + | names := make([]string, 0, len(parts)) | |||
| 267 | + | for _, part := range parts { | |||
| 268 | + | part = strings.TrimSpace(part) | |||
| 269 | + | // Each part is "name type" or "name type1/type2" — the first space separates name from type. | |||
| 270 | + | idx := strings.IndexByte(part, ' ') | |||
| 271 | + | if idx < 0 { | |||
| 272 | + | // No space means just a name with no type (shouldn't happen in valid Go, but handle it) | |||
| 273 | + | names = append(names, part) | |||
| 274 | + | } else { | |||
| 275 | + | names = append(names, part[:idx]) | |||
| 276 | + | } | |||
| 277 | + | } | |||
| 278 | + | return "(" + strings.Join(names, ", ") + ")" | |||
| 279 | + | } | |||
| 280 | + | ||||
| 256 | // filterSymbolsByGroup returns symbols that belong to the given group. | 281 | // filterSymbolsByGroup returns symbols that belong to the given group. | ||
| 257 | func filterSymbolsByGroup(symbols []model.Symbol, groupName string) []model.Symbol { | 282 | func filterSymbolsByGroup(symbols []model.Symbol, groupName string) []model.Symbol { | ||
| 258 | var result []model.Symbol | 283 | var result []model.Symbol | ||
| @@ -295,10 +320,11 @@ func toCamelCase(s string) string { | |||||
| 295 | // renderAdapters generates one thin adapter struct per group with forwarding methods. | 320 | // renderAdapters generates one thin adapter struct per group with forwarding methods. | ||
| 296 | func renderAdapters(m *model.Model) ([]byte, error) { | 321 | func renderAdapters(m *model.Model) ([]byte, error) { | ||
| 297 | type AdapterMethod struct { | 322 | type AdapterMethod struct { | ||
| 298 | - | CName string | 323 | + | CName string |
| 299 | - | GoName string | 324 | + | GoName string |
| 300 | - | Params string | 325 | + | Params string |
| 301 | - | Results string | 326 | + | CallArgs string |
| 327 | + | Results string | |||
| 302 | } | 328 | } | ||
| 303 | 329 | ||||
| 304 | type AdapterGroup struct { | 330 | type AdapterGroup struct { | ||
| @@ -315,10 +341,11 @@ func renderAdapters(m *model.Model) ([]byte, error) { | |||||
| 315 | for _, sym := range groupSyms { | 341 | for _, sym := range groupSyms { | ||
| 316 | params, results := parseSignature(sym.Signature) | 342 | params, results := parseSignature(sym.Signature) | ||
| 317 | methods = append(methods, AdapterMethod{ | 343 | methods = append(methods, AdapterMethod{ | ||
| 318 | - | CName: sym.Name, | 344 | + | CName: sym.Name, |
| 319 | - | GoName: toGoName(sym.Name), | 345 | + | GoName: toGoName(sym.Name), |
| 320 | - | Params: params, | 346 | + | Params: params, |
| 321 | - | Results: results, | 347 | + | CallArgs: extractParamNames(params), |
| 348 | + | Results: results, | |||
| 322 | }) | 349 | }) | ||
| 323 | if strings.Contains(sym.Signature, "unsafe.Pointer") { | 350 | if strings.Contains(sym.Signature, "unsafe.Pointer") { | ||
| 324 | needsUnsafe = true | 351 | needsUnsafe = true | ||
| @@ -348,26 +375,10 @@ func renderAdapters(m *model.Model) ([]byte, error) { | |||||
| 348 | 375 | ||||
| 349 | // renderComposite generates the composite CAPI interface that embeds all group interfaces. | 376 | // renderComposite generates the composite CAPI interface that embeds all group interfaces. | ||
| 350 | func renderComposite(m *model.Model) ([]byte, error) { | 377 | func renderComposite(m *model.Model) ([]byte, error) { | ||
| 351 | - | // Determine if any group uses unsafe.Pointer | |||
| 352 | - | needsUnsafe := false | |||
| 353 | - | for _, group := range m.Groups { | |||
| 354 | - | for _, sym := range filterSymbolsByGroup(m.Symbols, group.Name) { | |||
| 355 | - | if strings.Contains(sym.Signature, "unsafe.Pointer") { | |||
| 356 | - | needsUnsafe = true | |||
| 357 | - | break | |||
| 358 | - | } | |||
| 359 | - | } | |||
| 360 | - | if needsUnsafe { | |||
| 361 | - | break | |||
| 362 | - | } | |||
| 363 | - | } | |||
| 364 | - | ||||
| 365 | data := struct { | 378 | data := struct { | ||
| 366 | - | Groups []model.Group | 379 | + | Groups []model.Group |
| 367 | - | NeedsUnsafe bool | |||
| 368 | }{ | 380 | }{ | ||
| 369 | - | Groups: m.Groups, | 381 | + | Groups: m.Groups, |
| 370 | - | NeedsUnsafe: needsUnsafe, | |||
| 371 | } | 382 | } | ||
| 372 | 383 | ||||
| 373 | var buf bytes.Buffer | 384 | var buf bytes.Buffer | ||
| @@ -450,9 +461,9 @@ type {{$group.AdapterName}} struct{} | |||||
| 450 | {{range $group.Methods}} | 461 | {{range $group.Methods}} | ||
| 451 | func (a *{{$group.AdapterName}}) {{.GoName}}{{.Params}}{{if .Results}} {{.Results}}{{end}} { | 462 | func (a *{{$group.AdapterName}}) {{.GoName}}{{.Params}}{{if .Results}} {{.Results}}{{end}} { | ||
| 452 | {{- if .Results}} | 463 | {{- if .Results}} | ||
| 453 | - | return {{.CName}}{{.Params}} | 464 | + | return {{.CName}}{{.CallArgs}} |
| 454 | {{- else}} | 465 | {{- else}} | ||
| 455 | - | {{.CName}}{{.Params}} | 466 | + | {{.CName}}{{.CallArgs}} |
| 456 | {{- end}} | 467 | {{- end}} | ||
| 457 | } | 468 | } | ||
| 458 | {{end}} | 469 | {{end}} | ||
| @@ -462,9 +473,7 @@ func (a *{{$group.AdapterName}}) {{.GoName}}{{.Params}}{{if .Results}} {{.Result | |||||
| 462 | const compositeTemplate = `// Code generated by pipewiregen. DO NOT EDIT. | 473 | const compositeTemplate = `// Code generated by pipewiregen. DO NOT EDIT. | ||
| 463 | 474 | ||||
| 464 | package out | 475 | package out | ||
| 465 | - | {{if .NeedsUnsafe}} | 476 | + | |
| 466 | - | import "unsafe" | |||
| 467 | - | {{end}} | |||
| 468 | // CAPI is the composite interface that embeds all generated group interfaces. | 477 | // CAPI is the composite interface that embeds all generated group interfaces. | ||
| 469 | type CAPI interface { | 478 | type CAPI interface { | ||
| 470 | {{- range .Groups}} | 479 | {{- range .Groups}} | ||
M cmd/pipewiregen/internal/emitter/emitter_test.go +16 -0
| @@ -122,6 +122,18 @@ func TestEmitWritesAdaptersAndCompositeFiles(t *testing.T) { | |||
| 122 | 122 | t.Fatal("adapters output missing PWStreamConnect method") | |
| 123 | 123 | } | |
| 124 | 124 | ||
| 125 | + | // Verify adapter forwarding calls use argument names only (no types). | |
| 126 | + | // For pw_stream_connect with signature "func(stream unsafe.Pointer, direction int32, ...)", | |
| 127 | + | // the forwarding call must be "pw_stream_connect(stream, direction, id, flags, ports, n_ports)" | |
| 128 | + | // NOT "pw_stream_connect(stream unsafe.Pointer, direction int32, ...)" | |
| 129 | + | if strings.Contains(adapterStr, "pw_stream_connect(stream unsafe.Pointer") { | |
| 130 | + | t.Fatal("adapter forwarding call contains type annotations — call site should use only argument names") | |
| 131 | + | } | |
| 132 | + | // Verify correct call syntax: forwarding with just names | |
| 133 | + | if !strings.Contains(adapterStr, "pw_stream_connect(stream, direction, id, flags, ports, n_ports)") { | |
| 134 | + | t.Fatal("adapter forwarding call missing correct call-site syntax pw_stream_connect(stream, direction, id, flags, ports, n_ports)") | |
| 135 | + | } | |
| 136 | + | ||
| 125 | 137 | // Verify composite port file exists | |
| 126 | 138 | compositeContent, ok := paths["internal/ports/out/capi_gen.go"] | |
| 127 | 139 | if !ok { | |
| @@ -136,6 +148,10 @@ func TestEmitWritesAdaptersAndCompositeFiles(t *testing.T) { | |||
| 136 | 148 | if !strings.Contains(compositeStr, "StreamPlaybackAPI") { | |
| 137 | 149 | t.Fatal("composite output missing StreamPlaybackAPI embed") | |
| 138 | 150 | } | |
| 151 | + | // Verify composite does not contain unused unsafe import | |
| 152 | + | if strings.Contains(compositeStr, `import "unsafe"`) { | |
| 153 | + | t.Fatal("composite output must not contain unused unsafe import — composite only embeds interfaces") | |
| 154 | + | } | |
| 139 | 155 | } | |
| 140 | 156 | ||
| 141 | 157 | // testCleanupModel returns a model with cleanup symbols that should be generated. | |
| @@ -122,6 +122,18 @@ func TestEmitWritesAdaptersAndCompositeFiles(t *testing.T) { | |||||
| 122 | t.Fatal("adapters output missing PWStreamConnect method") | 122 | t.Fatal("adapters output missing PWStreamConnect method") | ||
| 123 | } | 123 | } | ||
| 124 | 124 | ||||
| 125 | + | // Verify adapter forwarding calls use argument names only (no types). | |||
| 126 | + | // For pw_stream_connect with signature "func(stream unsafe.Pointer, direction int32, ...)", | |||
| 127 | + | // the forwarding call must be "pw_stream_connect(stream, direction, id, flags, ports, n_ports)" | |||
| 128 | + | // NOT "pw_stream_connect(stream unsafe.Pointer, direction int32, ...)" | |||
| 129 | + | if strings.Contains(adapterStr, "pw_stream_connect(stream unsafe.Pointer") { | |||
| 130 | + | t.Fatal("adapter forwarding call contains type annotations — call site should use only argument names") | |||
| 131 | + | } | |||
| 132 | + | // Verify correct call syntax: forwarding with just names | |||
| 133 | + | if !strings.Contains(adapterStr, "pw_stream_connect(stream, direction, id, flags, ports, n_ports)") { | |||
| 134 | + | t.Fatal("adapter forwarding call missing correct call-site syntax pw_stream_connect(stream, direction, id, flags, ports, n_ports)") | |||
| 135 | + | } | |||
| 136 | + | ||||
| 125 | // Verify composite port file exists | 137 | // Verify composite port file exists | ||
| 126 | compositeContent, ok := paths["internal/ports/out/capi_gen.go"] | 138 | compositeContent, ok := paths["internal/ports/out/capi_gen.go"] | ||
| 127 | if !ok { | 139 | if !ok { | ||
| @@ -136,6 +148,10 @@ func TestEmitWritesAdaptersAndCompositeFiles(t *testing.T) { | |||||
| 136 | if !strings.Contains(compositeStr, "StreamPlaybackAPI") { | 148 | if !strings.Contains(compositeStr, "StreamPlaybackAPI") { | ||
| 137 | t.Fatal("composite output missing StreamPlaybackAPI embed") | 149 | t.Fatal("composite output missing StreamPlaybackAPI embed") | ||
| 138 | } | 150 | } | ||
| 151 | + | // Verify composite does not contain unused unsafe import | |||
| 152 | + | if strings.Contains(compositeStr, `import "unsafe"`) { | |||
| 153 | + | t.Fatal("composite output must not contain unused unsafe import — composite only embeds interfaces") | |||
| 154 | + | } | |||
| 139 | } | 155 | } | ||
| 140 | 156 | ||||
| 141 | // testCleanupModel returns a model with cleanup symbols that should be generated. | 157 | // testCleanupModel returns a model with cleanup symbols that should be generated. | ||