Requested changes of #3

1. defer the close of `archiveFile` and `src`
2. use `log.Printf` to replace `fmt.Printf` and `fmt.Fprintf`
3. `panic()` if the log directory does not exist and can not be
created either
4. fetching the previous writer and close it if valid

src/ganted2radius.go

@@ -90,23 +90,24 @@ func archiveLogs(logDir string, maxBackup int) error {
 		if err != nil {
 			return err
 		}
+		defer archiveFile.Close()
 		// concatenate `maxBackup` access-<datetime>.log files to access-<date>.log
 		for _, logFile := range logFiles {
 			src, err := os.Open(logFile)
 			if err != nil {
 				return err
 			}
+			defer src.Close()
 			_, err = io.Copy(archiveFile, src)
 			if err != nil {
 				return err
 			}
-			src.Close()
+
 		}
 		// compress access-<date>.log
 		if err := compressFile(archiveFilePath); err != nil {
 			return err
 		}
-		archiveFile.Close()
 		// If the archiveFile exists, some error occur, and archiveFile need to delete
 		// As the compressFile will remove the original archiveFile
 		// Else, everything is ok, delete the original log files
@@ -139,19 +140,19 @@ func parseLogFile(filename string) (map[string]int, error) {
 		line := scanner.Text()
 		fields := strings.Fields(line)
 		if len(fields) != 8 {
-			fmt.Printf("Skipping malformed line: %s\n", line)
+			log.Printf("Skipping malformed line: %s\n", line)
 			continue
 		}
 
 		identity := fields[3]
 		bytesIn, err := strconv.Atoi(fields[6])
 		if err != nil {
-			fmt.Printf("Error parsing bytes in: %v\n", err)
+			log.Printf("Error parsing bytes in: %v\n", err)
 			continue
 		}
 		bytesOut, err := strconv.Atoi(fields[7])
 		if err != nil {
-			fmt.Printf("Error parsing bytes out: %v\n", err)
+			log.Printf("Error parsing bytes out: %v\n", err)
 			continue
 		}
 		totalBytes := bytesIn + bytesOut
@@ -165,15 +166,15 @@ func parseLogFile(filename string) (map[string]int, error) {
 func (r *RadiusCredentials) sendAccountingData(identity string, bytes int) error {
 	// send an CodeAccessRequest for test
 	sessionID := strconv.FormatInt(time.Now().Unix(), 10)
-	fmt.Printf("Sending accounting data for identity %s, session ID %s, bytes %d\n", identity, sessionID, bytes)
+	log.Printf("Sending accounting data for identity %s, session ID %s, bytes %d\n", identity, sessionID, bytes)
 
 	// Send start accounting packet
 	startPacket := radius.New(radius.CodeAccountingRequest, []byte(r.Secret))
 	rfc2865.UserName_SetString(startPacket, identity)
 	rfc2865.NASIdentifier_SetString(startPacket, r.NASIdentifier)
 	rfc2866.AcctSessionID_Set(startPacket, []byte(sessionID))
 	rfc2866.AcctStatusType_Set(startPacket, rfc2866.AcctStatusType_Value_Start)
-	fmt.Printf("Sending start packet\n")
+	// log.Printf("Sending start packet\n")
 
 	startReply, err := radius.Exchange(context.Background(), startPacket, r.AccountingServer)
 	if err != nil {
@@ -182,7 +183,7 @@ func (r *RadiusCredentials) sendAccountingData(identity string, bytes int) error
 	if startReply.Code != radius.CodeAccountingResponse {
 		return fmt.Errorf("unexpected response from RADIUS server")
 	}
-	fmt.Printf("Received start reply\n")
+	// log.Printf("Received start reply\n")
 
 	// Send stop accounting packet
 	stopPacket := radius.New(radius.CodeAccountingRequest, r.Secret)
@@ -191,7 +192,7 @@ func (r *RadiusCredentials) sendAccountingData(identity string, bytes int) error
 	rfc2866.AcctSessionID_SetString(stopPacket, sessionID)
 	rfc2866.AcctStatusType_Set(stopPacket, rfc2866.AcctStatusType_Value_Stop)
 	rfc2866.AcctOutputOctets_Set(stopPacket, rfc2866.AcctOutputOctets(bytes))
-	fmt.Printf("Sending stop packet\n")
+	// log.Printf("Sending stop packet\n")
 
 	stopReply, err := radius.Exchange(context.Background(), stopPacket, r.AccountingServer)
 	if err != nil {
@@ -200,7 +201,7 @@ func (r *RadiusCredentials) sendAccountingData(identity string, bytes int) error
 	if stopReply.Code != radius.CodeAccountingResponse {
 		return fmt.Errorf("unexpected response from RADIUS server")
 	}
-	fmt.Printf("Received stop reply\n")
+	// log.Printf("Received stop reply\n")
 
 	return nil
 }
@@ -226,16 +227,16 @@ func (r *RadiusCredentials) accounting(accessLogger *log.Logger) error {
 	}
 	stats, err := parseLogFile(accountingLogFile)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "Error parsing log file: %v\n", err)
+		log.Printf("[ERR] Failed to parse log file %s: %v\n", accountingLogFile, err)
 		return err
 	}
 	// Sending accounting data
 	for identity, bytes := range stats {
 		err := r.sendAccountingData(identity, bytes)
 		if err != nil {
-			fmt.Fprintf(os.Stderr, "Error sending accounting data for identity %s: %v\n", identity, err)
+			log.Printf("[ERR] Failed to send accounting data for identity %s: %v\n", identity, err)
 		} else {
-			fmt.Printf("Sent accounting data for identity %s\n", identity)
+			log.Printf("Sent accounting data for identity %s\n", identity)
 		}
 	}
 	// Compress all access-<datetime>.log files in the log directory

src/main.go

@@ -132,7 +132,6 @@ func (r *RadiusCredentials) accountingCron(accessLogger, errorLogger *log.Logger
 	})
 	if err != nil {
 		log.Fatalf("[ERR] Add accounting cron job: %s", err)
-		return nil
 	}
 	c.Start()
 	return c
@@ -157,20 +156,25 @@ func init() {
 	// init dir GANTED_LOG_DIR
 	gantedLogDir := getEnv("GANTED_LOG_DIR", "/var/log/ganted")
 	if _, err := os.Stat(gantedLogDir); os.IsNotExist(err) {
-		os.Mkdir(gantedLogDir, 0755)
+		if err := os.Mkdir(gantedLogDir, 0755); err != nil {
+			panic(err)
+		}
 	}
 }
 
 func setFileLoggerOutput(logger *log.Logger, filePath string) error {
 	if filePath == "" {
 		return nil
 	}
-	file, err := os.OpenFile(filePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644)
+	file, err := os.OpenFile(filePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o644)
 	if err != nil {
 		log.Fatalf("[ERR] Open log file: %s", err)
-		return err
 	}
+	prevWriter := logger.Writer()
 	logger.SetOutput(file)
+	if closer, ok := prevWriter.(io.Closer); ok {
+		closer.Close()
+	}
 	return nil
 }