From 55c7dca619109e6feadf9f786317efa7640d0e9c Mon Sep 17 00:00:00 2001
From: Daniel Grau <dgrau@google.com>
Date: Thu, 28 Sep 2023 16:42:31 +0000
Subject: [PATCH] feedback

---
 dataplane/standalone/apigen/ccgen/ccgen.go         | 10 ++++++----
 dataplane/standalone/apigen/docparser/docparser.go |  6 +++---
 dataplane/standalone/apigen/saiast/saiast.go       |  1 +
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/dataplane/standalone/apigen/ccgen/ccgen.go b/dataplane/standalone/apigen/ccgen/ccgen.go
index 1465b472..b0e9dc7d 100644
--- a/dataplane/standalone/apigen/ccgen/ccgen.go
+++ b/dataplane/standalone/apigen/ccgen/ccgen.go
@@ -39,10 +39,10 @@ func Generate(doc *docparser.SAIInfo, sai *saiast.SAIAPI) (map[string]string, er
 			APIName:      apiName,
 			ProtoInclude: apiName + ".pb",
 		}
-		if apiName == "switch" {
+		switch apiName {
+		case "switch":
 			ccData.Globals = append(ccData.Globals, "std::unique_ptr<PortStateReactor> port_state;")
-		}
-		if apiName == "hostif" {
+		case "hostif":
 			ccData.Globals = append(ccData.Globals, "int nextIdx = 1;")
 		}
 		for _, fn := range iface.Funcs {
@@ -76,7 +76,9 @@ func sanitizeProtoName(inName string) string {
 	return name
 }
 
-// createCCData returns a struct with the template data for the given function.
+// createCCData returns a two structs with the template data for the given function.
+// The first is the implementation of the API: CreateFoo.
+// The second is the a conversion func from attribute list to the proto message. covert_create_foo.
 func createCCData(meta *saiast.FuncMetadata, apiName string, sai *saiast.SAIAPI, info *docparser.SAIInfo, fn *saiast.TypeDecl) (*templateFunc, *templateFunc) {
 	if info.Attrs[meta.TypeName] == nil {
 		fmt.Printf("no doc info for type: %v\n", meta.TypeName)
diff --git a/dataplane/standalone/apigen/docparser/docparser.go b/dataplane/standalone/apigen/docparser/docparser.go
index b5d488e7..888edffd 100644
--- a/dataplane/standalone/apigen/docparser/docparser.go
+++ b/dataplane/standalone/apigen/docparser/docparser.go
@@ -27,9 +27,9 @@ import (
 
 // SAIInfo contains all the info parsed from the doxygen.
 type SAIInfo struct {
-	// attrs is a map from sai type (sai_port_t) to its attributes.
+	// Attrs is a map from sai type (sai_port_t) to its attributes.
 	Attrs map[string]*Attr
-	// attrs is a map from enum name (sai_port_media_type_t) to the values of the enum.
+	// Enums is a map from enum name (sai_port_media_type_t) to the values of the enum.
 	Enums map[string][]*Enum
 }
 
@@ -173,7 +173,7 @@ func memberToAttrInfo(enum MemberDef) *Attr {
 
 func memberToEnumValueStrings(enum MemberDef) []*Enum {
 	res := []*Enum{}
-	prev := -1
+	prev := -1 // Since enum values may repeat, store previous enum value. If the enum value isn't an assignment, then it is prev + 1.
 	for i, value := range enum.EnumValues {
 		val := prev + 1
 		value.Initializer = strings.TrimPrefix(value.Initializer, "= ")
diff --git a/dataplane/standalone/apigen/saiast/saiast.go b/dataplane/standalone/apigen/saiast/saiast.go
index 4b389c65..2decbffe 100644
--- a/dataplane/standalone/apigen/saiast/saiast.go
+++ b/dataplane/standalone/apigen/saiast/saiast.go
@@ -126,6 +126,7 @@ func (sai *SAIAPI) GetFuncMeta(fn *TypeDecl) *FuncMetadata {
 		if strings.Contains(param.Typ, "entry") {
 			meta.Entry = TrimSAIName(strings.TrimPrefix(param.Typ, "const "), true, false)
 		}
+		// The switch_id is either the first or second arguments to function.
 		if (i == 0 || i == 1) && param.Name == "switch_id" {
 			meta.IsSwitchScoped = true
 		}