Skip to content

fix: Address critical code review issues #1

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
SQReder merged 1 commit into from
Oct 22, 2025
Merged

fix: Address critical code review issues #1

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
123 changes: 95 additions & 28 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//go:build windows

package main

import (
Expand Down Expand Up @@ -30,6 +32,7 @@ func (d Device) listInstances() []DeviceInstance {
hidPath := joinPath(rootHidPath, d.id)
hidKey, err := registry.OpenKey(registry.LOCAL_MACHINE, hidPath, registry.READ)
panicOnError(err, fmt.Sprintf("HID %s subkey", hidPath))
defer hidKey.Close()

hidSubKeys := listSubKeysNames(hidKey)

Expand All @@ -42,30 +45,50 @@ func (d Device) listInstances() []DeviceInstance {
}
subDevicePath := joinPath(hidPath, hidSubKey)
subDeviceKey, err := registry.OpenKey(registry.LOCAL_MACHINE, subDevicePath, registry.READ)
panicOnError(err, "Nya")
if err != nil {
WarningLogger.Printf("Failed to open registry key %s: %v", subDevicePath, err)
continue
}
defer subDeviceKey.Close()

desc, _, err := subDeviceKey.GetStringValue("DeviceDesc")
if err != nil {
WarningLogger.Printf("Failed to read DeviceDesc for %s: %v", subDevicePath, err)
subDeviceKey.Close() // Close early before continue
continue
}

subDevice.desc = parseDeviceDesc(desc)
parsedDesc, err := parseDeviceDesc(desc)
if err != nil {
WarningLogger.Printf("Failed to parse device description for %s: %v", subDevicePath, err)
subDeviceKey.Close() // Close early before continue
continue
}
subDevice.desc = parsedDesc

result = append(result, subDevice)
}

return result
}

func parseDeviceDesc(desc string) deviceDesc {
func parseDeviceDesc(desc string) (deviceDesc, error) {
commaIndex := strings.Index(desc, ",")
colonIdex := strings.Index(desc, ";")
colonIndex := strings.Index(desc, ";")

// Validate indices to prevent panic
if commaIndex == -1 || colonIndex == -1 || commaIndex >= colonIndex {
return deviceDesc{}, fmt.Errorf("invalid device description format: %q", desc)
}

driver := desc[:commaIndex]
name := desc[colonIdex+1:]
name := desc[colonIndex+1:]

return deviceDesc{
driver: driver,
deviceType: desc[commaIndex+1 : colonIdex],
deviceType: desc[commaIndex+1 : colonIndex],
name: name,
}
}, nil
}

func (d deviceDesc) isMouse() bool {
Expand All @@ -76,6 +99,7 @@ func listRootDevices() []Device {
DebugLogger.Printf("Open %s", rootHidPath)
key, err := registry.OpenKey(registry.LOCAL_MACHINE, rootHidPath, registry.READ)
panicOnError(err, "Open HID key")
defer key.Close()

rootNames := listSubKeysNames(key)

Expand All @@ -98,6 +122,7 @@ func (d *DeviceInstance) isFlippable() bool {
deviceParamsPath := d.getDeviceParamsPath()
key, err := registry.OpenKey(registry.LOCAL_MACHINE, deviceParamsPath, registry.READ)
panicOnError(err, "Open Device parameters")
defer key.Close()

stat, err := key.Stat()
panicOnError(err, "Device parameters stat")
Expand All @@ -122,37 +147,46 @@ func (d *DeviceInstance) getDeviceParamsPath() string {

func (d DeviceInstance) getWheelDirection() int {
if !d.desc.isMouse() {
log.Println("Skip", strconv.Quote(d.getFriendlyName()), "- it's not a mouse")
DebugLogger.Printf("Skip %s - it's not a mouse", strconv.Quote(d.getFriendlyName()))
return WHEEL_UNKNOWN
}

defer func() {
if err := recover(); err != nil {
log.Println("panic occurred:", err)
}
}()

parametersPath := joinPath(rootHidPath, d.parent.id, d.id, "Device Parameters")
key, err := registry.OpenKey(registry.LOCAL_MACHINE, parametersPath, registry.READ)
panicOnError(err, "Open Device parameters")
if err != nil {
WarningLogger.Printf("Failed to open Device Parameters for %s: %v", d.getFriendlyName(), err)
return WHEEL_UNKNOWN
}
defer key.Close()

stat, err := key.Stat()
panicOnError(err, "Device Parameters stat")
if err != nil {
WarningLogger.Printf("Failed to get Device Parameters stat for %s: %v", d.getFriendlyName(), err)
return WHEEL_UNKNOWN
}
valueCount := stat.ValueCount

if valueCount == 0 {
return WHEEL_UNKNOWN
}

valueNames, err := key.ReadValueNames(int(valueCount))
if err != nil {
WarningLogger.Printf("Failed to read value names for %s: %v", d.getFriendlyName(), err)
return WHEEL_UNKNOWN
}

if !contains(valueNames, KeyNameFlipFlopWheel) {
return WHEEL_UNKNOWN
}

wheelFlipFlag, _, err := key.GetIntegerValue(KeyNameFlipFlopWheel)
panicOnError(err, "FlipFlopWheel value read")
if err != nil {
WarningLogger.Printf("Failed to read FlipFlopWheel value for %s: %v", d.getFriendlyName(), err)
return WHEEL_UNKNOWN
}

DebugLogger.Println("wheelFlipFlag", wheelFlipFlag)
DebugLogger.Printf("wheelFlipFlag for %s: %d", d.getFriendlyName(), wheelFlipFlag)

return int(wheelFlipFlag)
}
Expand All @@ -164,24 +198,44 @@ func main() {

configurable := loadDeviceInstances(rootDevices)

if len(configurable) == 0 {
fmt.Println("No configurable mouse devices found.")
fmt.Println("This could mean:")
fmt.Println(" - No mice are connected")
fmt.Println(" - The program doesn't have administrator privileges")
fmt.Println(" - Your mice don't support the FlipFlopWheel registry setting")
ErrorLogger.Panic("No configurable devices found")
}

printDevicesTable(configurable)

fmt.Println("Which device you want to flip?")
fmt.Print("Please enter the index: ")

scanner := bufio.NewScanner(os.Stdin)
scanner.Scan()
if !scanner.Scan() {
if err := scanner.Err(); err != nil {
ErrorLogger.Panicf("Failed to read input: %v", err)
}
ErrorLogger.Panic("No input provided")
}

line := scanner.Text()
trimmedInput := strings.TrimSpace(line)

trimmedInput := strings.Trim(string(line), "\n")
if trimmedInput == "" {
ErrorLogger.Panic("Empty input provided")
}

DebugLogger.Println("Line", trimmedInput)
DebugLogger.Println("User input:", trimmedInput)

instanceIndex, err := strconv.Atoi(trimmedInput)
panicOnError(err, "Convert string to index")
if err != nil {
ErrorLogger.Panicf("Invalid index format: %v", err)
}

if instanceIndex < 0 || instanceIndex >= len(configurable) {
ErrorLogger.Panicf("Unknown index. Expected 0..%d, but receive %d", len(configurable)-1, instanceIndex)
ErrorLogger.Panicf("Index out of range. Expected 0..%d, but received %d", len(configurable)-1, instanceIndex)
}

instance := configurable[instanceIndex]
Expand All @@ -192,19 +246,32 @@ func toggleWheel(instance DeviceInstance) {
currentDirection := instance.getWheelDirection()

if currentDirection != WHEEL_FLIPPED && currentDirection != WHEEL_NORMAL {
ErrorLogger.Panicln("Unknown current direction", instance)
ErrorLogger.Panicf("Cannot toggle wheel for %s: current direction is unknown or unsupported (value: %d)",
instance.getFriendlyName(), currentDirection)
}

path := joinPath(rootHidPath, instance.GetDevicePath(), "Device Parameters")
DebugLogger.Println("Looking for key", path)
DebugLogger.Printf("Opening registry key for writing: %s", path)
key, err := registry.OpenKey(registry.LOCAL_MACHINE, path, registry.WRITE)
panicOnError(err, "Open instance for writing")
if err != nil {
ErrorLogger.Panicf("Failed to open registry key for writing: %v", err)
}
defer key.Close()

flippedValue := currentDirection ^ 1
DebugLogger.Println("New value is", flippedValue)
DebugLogger.Printf("Changing wheel direction from %d to %d for %s",
currentDirection, flippedValue, instance.getFriendlyName())

err = key.SetDWordValue("FlipFlopWheel", uint32(flippedValue))
panicOnError(err, "Set new value")
if err != nil {
ErrorLogger.Panicf("Failed to set FlipFlopWheel value: %v", err)
}

fmt.Printf("\nSuccessfully changed wheel direction for %s from %s to %s\n",
instance.getFriendlyName(),
WheelDirectionToString(currentDirection),
WheelDirectionToString(flippedValue))
fmt.Println("Please reconnect your mouse or restart your computer for changes to take effect.")
}

func loadDeviceInstances(devices []Device) []DeviceInstance {
Expand Down