Skip to content

Commit 2c3ebed

Browse files
kolyshkinclaude
authored andcommitted
dbus: dedup result conversion code
It looks like listUnitsInternal performs an unnecessary conversion from [][]any to []any. Also, its code has been copied over a few times, with the only change to resulting type. Introduce generic helper functions convertSlice and storeSlice to eliminate duplicated code. Also, remove unnecessary intermediate slice allocation and conversion. This change: - Adds convertSlice[T] to convert []any to typed slices - Adds storeSlice[T] to fetch and convert D-Bus results - Updates 9 functions to use the new helpers - Removes ~150 lines of duplicated code - Eliminates listUnitsInternal and listUnitFilesInternal wrappers All functionality is preserved and tests pass. Co-Authored-By: Claude Sonnet 4.5 <[email protected]> Signed-off-by: Kir Kolyshkin <[email protected]>
1 parent aac8e00 commit 2c3ebed

File tree

1 file changed

+31
-177
lines changed

1 file changed

+31
-177
lines changed

dbus/methods.go

Lines changed: 31 additions & 177 deletions
Original file line numberDiff line numberDiff line change
@@ -397,30 +397,33 @@ type UnitStatus struct {
397397

398398
type storeFunc func(retvalues ...any) error
399399

400-
func (c *Conn) listUnitsInternal(f storeFunc) ([]UnitStatus, error) {
401-
result := make([][]any, 0)
402-
err := f(&result)
403-
if err != nil {
404-
return nil, err
400+
// convertSlice converts a []any result into a slice of the target type T
401+
// using dbus.Store to handle the type conversion.
402+
func convertSlice[T any](result []any) ([]T, error) {
403+
converted := make([]T, len(result))
404+
convertedInterface := make([]any, len(converted))
405+
for i := range converted {
406+
convertedInterface[i] = &converted[i]
405407
}
406408

407-
resultInterface := make([]any, len(result))
408-
for i := range result {
409-
resultInterface[i] = result[i]
409+
err := dbus.Store(result, convertedInterface...)
410+
if err != nil {
411+
return nil, err
410412
}
411413

412-
status := make([]UnitStatus, len(result))
413-
statusInterface := make([]any, len(status))
414-
for i := range status {
415-
statusInterface[i] = &status[i]
416-
}
414+
return converted, nil
415+
}
417416

418-
err = dbus.Store(resultInterface, statusInterface...)
417+
// storeSlice fetches D-Bus array results via the provided storeFunc
418+
// and converts them into a slice of the target type T.
419+
func storeSlice[T any](f storeFunc) ([]T, error) {
420+
var result []any
421+
err := f(&result)
419422
if err != nil {
420423
return nil, err
421424
}
422425

423-
return status, nil
426+
return convertSlice[T](result)
424427
}
425428

426429
// GetUnitByPID returns the unit object path of the unit a process ID
@@ -457,7 +460,7 @@ func (c *Conn) ListUnits() ([]UnitStatus, error) {
457460
// Also note that a unit is only loaded if it is active and/or enabled.
458461
// Units that are both disabled and inactive will thus not be returned.
459462
func (c *Conn) ListUnitsContext(ctx context.Context) ([]UnitStatus, error) {
460-
return c.listUnitsInternal(c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.ListUnits", 0).Store)
463+
return storeSlice[UnitStatus](c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.ListUnits", 0).Store)
461464
}
462465

463466
// Deprecated: use ListUnitsFilteredContext instead.
@@ -468,7 +471,7 @@ func (c *Conn) ListUnitsFiltered(states []string) ([]UnitStatus, error) {
468471
// ListUnitsFilteredContext returns an array with units filtered by state.
469472
// It takes a list of units' statuses to filter.
470473
func (c *Conn) ListUnitsFilteredContext(ctx context.Context, states []string) ([]UnitStatus, error) {
471-
return c.listUnitsInternal(c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.ListUnitsFiltered", 0, states).Store)
474+
return storeSlice[UnitStatus](c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.ListUnitsFiltered", 0, states).Store)
472475
}
473476

474477
// Deprecated: use ListUnitsByPatternsContext instead.
@@ -481,7 +484,7 @@ func (c *Conn) ListUnitsByPatterns(states []string, patterns []string) ([]UnitSt
481484
// Note that units may be known by multiple names at the same time,
482485
// and hence there might be more unit names loaded than actual units behind them.
483486
func (c *Conn) ListUnitsByPatternsContext(ctx context.Context, states []string, patterns []string) ([]UnitStatus, error) {
484-
return c.listUnitsInternal(c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.ListUnitsByPatterns", 0, states, patterns).Store)
487+
return storeSlice[UnitStatus](c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.ListUnitsByPatterns", 0, states, patterns).Store)
485488
}
486489

487490
// Deprecated: use ListUnitsByNamesContext instead.
@@ -496,48 +499,22 @@ func (c *Conn) ListUnitsByNames(units []string) ([]UnitStatus, error) {
496499
//
497500
// Requires systemd v230 or higher.
498501
func (c *Conn) ListUnitsByNamesContext(ctx context.Context, units []string) ([]UnitStatus, error) {
499-
return c.listUnitsInternal(c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.ListUnitsByNames", 0, units).Store)
502+
return storeSlice[UnitStatus](c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.ListUnitsByNames", 0, units).Store)
500503
}
501504

502505
type UnitFile struct {
503506
Path string
504507
Type string
505508
}
506509

507-
func (c *Conn) listUnitFilesInternal(f storeFunc) ([]UnitFile, error) {
508-
result := make([][]any, 0)
509-
err := f(&result)
510-
if err != nil {
511-
return nil, err
512-
}
513-
514-
resultInterface := make([]any, len(result))
515-
for i := range result {
516-
resultInterface[i] = result[i]
517-
}
518-
519-
files := make([]UnitFile, len(result))
520-
fileInterface := make([]any, len(files))
521-
for i := range files {
522-
fileInterface[i] = &files[i]
523-
}
524-
525-
err = dbus.Store(resultInterface, fileInterface...)
526-
if err != nil {
527-
return nil, err
528-
}
529-
530-
return files, nil
531-
}
532-
533510
// Deprecated: use ListUnitFilesContext instead.
534511
func (c *Conn) ListUnitFiles() ([]UnitFile, error) {
535512
return c.ListUnitFilesContext(context.Background())
536513
}
537514

538515
// ListUnitFilesContext returns an array of all available units on disk.
539516
func (c *Conn) ListUnitFilesContext(ctx context.Context) ([]UnitFile, error) {
540-
return c.listUnitFilesInternal(c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.ListUnitFiles", 0).Store)
517+
return storeSlice[UnitFile](c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.ListUnitFiles", 0).Store)
541518
}
542519

543520
// Deprecated: use ListUnitFilesByPatternsContext instead.
@@ -547,7 +524,7 @@ func (c *Conn) ListUnitFilesByPatterns(states []string, patterns []string) ([]Un
547524

548525
// ListUnitFilesByPatternsContext returns an array of all available units on disk matched the patterns.
549526
func (c *Conn) ListUnitFilesByPatternsContext(ctx context.Context, states []string, patterns []string) ([]UnitFile, error) {
550-
return c.listUnitFilesInternal(c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.ListUnitFilesByPatterns", 0, states, patterns).Store)
527+
return storeSlice[UnitFile](c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.ListUnitFilesByPatterns", 0, states, patterns).Store)
551528
}
552529

553530
type LinkUnitFileChange EnableUnitFileChange
@@ -575,29 +552,7 @@ func (c *Conn) LinkUnitFiles(files []string, runtime bool, force bool) ([]LinkUn
575552
// or unlink), the file name of the symlink and the destination of the
576553
// symlink.
577554
func (c *Conn) LinkUnitFilesContext(ctx context.Context, files []string, runtime bool, force bool) ([]LinkUnitFileChange, error) {
578-
result := make([][]any, 0)
579-
err := c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.LinkUnitFiles", 0, files, runtime, force).Store(&result)
580-
if err != nil {
581-
return nil, err
582-
}
583-
584-
resultInterface := make([]any, len(result))
585-
for i := range result {
586-
resultInterface[i] = result[i]
587-
}
588-
589-
changes := make([]LinkUnitFileChange, len(result))
590-
changesInterface := make([]any, len(changes))
591-
for i := range changes {
592-
changesInterface[i] = &changes[i]
593-
}
594-
595-
err = dbus.Store(resultInterface, changesInterface...)
596-
if err != nil {
597-
return nil, err
598-
}
599-
600-
return changes, nil
555+
return storeSlice[LinkUnitFileChange](c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.LinkUnitFiles", 0, files, runtime, force).Store)
601556
}
602557

603558
// Deprecated: use EnableUnitFilesContext instead.
@@ -623,25 +578,14 @@ func (c *Conn) EnableUnitFiles(files []string, runtime bool, force bool) (bool,
623578
// symlink.
624579
func (c *Conn) EnableUnitFilesContext(ctx context.Context, files []string, runtime bool, force bool) (bool, []EnableUnitFileChange, error) {
625580
var carries_install_info bool
581+
var result []any
626582

627-
result := make([][]any, 0)
628583
err := c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.EnableUnitFiles", 0, files, runtime, force).Store(&carries_install_info, &result)
629584
if err != nil {
630585
return false, nil, err
631586
}
632587

633-
resultInterface := make([]any, len(result))
634-
for i := range result {
635-
resultInterface[i] = result[i]
636-
}
637-
638-
changes := make([]EnableUnitFileChange, len(result))
639-
changesInterface := make([]any, len(changes))
640-
for i := range changes {
641-
changesInterface[i] = &changes[i]
642-
}
643-
644-
err = dbus.Store(resultInterface, changesInterface...)
588+
changes, err := convertSlice[EnableUnitFileChange](result)
645589
if err != nil {
646590
return false, nil, err
647591
}
@@ -673,29 +617,7 @@ func (c *Conn) DisableUnitFiles(files []string, runtime bool) ([]DisableUnitFile
673617
// symlink or unlink), the file name of the symlink and the destination of the
674618
// symlink.
675619
func (c *Conn) DisableUnitFilesContext(ctx context.Context, files []string, runtime bool) ([]DisableUnitFileChange, error) {
676-
result := make([][]any, 0)
677-
err := c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.DisableUnitFiles", 0, files, runtime).Store(&result)
678-
if err != nil {
679-
return nil, err
680-
}
681-
682-
resultInterface := make([]any, len(result))
683-
for i := range result {
684-
resultInterface[i] = result[i]
685-
}
686-
687-
changes := make([]DisableUnitFileChange, len(result))
688-
changesInterface := make([]any, len(changes))
689-
for i := range changes {
690-
changesInterface[i] = &changes[i]
691-
}
692-
693-
err = dbus.Store(resultInterface, changesInterface...)
694-
if err != nil {
695-
return nil, err
696-
}
697-
698-
return changes, nil
620+
return storeSlice[DisableUnitFileChange](c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.DisableUnitFiles", 0, files, runtime).Store)
699621
}
700622

701623
type DisableUnitFileChange struct {
@@ -719,29 +641,7 @@ func (c *Conn) MaskUnitFiles(files []string, runtime bool, force bool) ([]MaskUn
719641
// runtime only (true, /run/systemd/..), or persistently (false,
720642
// /etc/systemd/..).
721643
func (c *Conn) MaskUnitFilesContext(ctx context.Context, files []string, runtime bool, force bool) ([]MaskUnitFileChange, error) {
722-
result := make([][]any, 0)
723-
err := c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.MaskUnitFiles", 0, files, runtime, force).Store(&result)
724-
if err != nil {
725-
return nil, err
726-
}
727-
728-
resultInterface := make([]any, len(result))
729-
for i := range result {
730-
resultInterface[i] = result[i]
731-
}
732-
733-
changes := make([]MaskUnitFileChange, len(result))
734-
changesInterface := make([]any, len(changes))
735-
for i := range changes {
736-
changesInterface[i] = &changes[i]
737-
}
738-
739-
err = dbus.Store(resultInterface, changesInterface...)
740-
if err != nil {
741-
return nil, err
742-
}
743-
744-
return changes, nil
644+
return storeSlice[MaskUnitFileChange](c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.MaskUnitFiles", 0, files, runtime, force).Store)
745645
}
746646

747647
type MaskUnitFileChange struct {
@@ -763,29 +663,7 @@ func (c *Conn) UnmaskUnitFiles(files []string, runtime bool) ([]UnmaskUnitFileCh
763663
// for runtime only (true, /run/systemd/..), or persistently (false,
764664
// /etc/systemd/..).
765665
func (c *Conn) UnmaskUnitFilesContext(ctx context.Context, files []string, runtime bool) ([]UnmaskUnitFileChange, error) {
766-
result := make([][]any, 0)
767-
err := c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.UnmaskUnitFiles", 0, files, runtime).Store(&result)
768-
if err != nil {
769-
return nil, err
770-
}
771-
772-
resultInterface := make([]any, len(result))
773-
for i := range result {
774-
resultInterface[i] = result[i]
775-
}
776-
777-
changes := make([]UnmaskUnitFileChange, len(result))
778-
changesInterface := make([]any, len(changes))
779-
for i := range changes {
780-
changesInterface[i] = &changes[i]
781-
}
782-
783-
err = dbus.Store(resultInterface, changesInterface...)
784-
if err != nil {
785-
return nil, err
786-
}
787-
788-
return changes, nil
666+
return storeSlice[UnmaskUnitFileChange](c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.UnmaskUnitFiles", 0, files, runtime).Store)
789667
}
790668

791669
type UnmaskUnitFileChange struct {
@@ -831,31 +709,7 @@ func (c *Conn) ListJobs() ([]JobStatus, error) {
831709

832710
// ListJobsContext returns an array with all currently queued jobs.
833711
func (c *Conn) ListJobsContext(ctx context.Context) ([]JobStatus, error) {
834-
return c.listJobsInternal(ctx)
835-
}
836-
837-
func (c *Conn) listJobsInternal(ctx context.Context) ([]JobStatus, error) {
838-
result := make([][]any, 0)
839-
if err := c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.ListJobs", 0).Store(&result); err != nil {
840-
return nil, err
841-
}
842-
843-
resultInterface := make([]any, len(result))
844-
for i := range result {
845-
resultInterface[i] = result[i]
846-
}
847-
848-
status := make([]JobStatus, len(result))
849-
statusInterface := make([]any, len(status))
850-
for i := range status {
851-
statusInterface[i] = &status[i]
852-
}
853-
854-
if err := dbus.Store(resultInterface, statusInterface...); err != nil {
855-
return nil, err
856-
}
857-
858-
return status, nil
712+
return storeSlice[JobStatus](c.sysobj.CallWithContext(ctx, "org.freedesktop.systemd1.Manager.ListJobs", 0).Store)
859713
}
860714

861715
// FreezeUnit freezes the cgroup associated with the unit.

0 commit comments

Comments
 (0)