Skip to content

Conversation

@nleiva
Copy link
Contributor

@nleiva nleiva commented Jul 22, 2025

Fixes a bug where using skip_obsolete or skip_deprecated flags causes a nil pointer dereference during code generation for YANG lists with obsolete keys (#1037).

ERROR Generating GoStruct Code: template: keyHelper:4:37: executing "keyHelper" at <$key.IsPtr>: nil pointer evaluating *gogen.yangFieldMap.IsPtr

When skip_obsolete is enabled, obsolete fields are filtered out during IR generation in getOrderedDirDetails, removing them from ParsedDirectory.Fields. However, ParsedDirectory.ListKeys retains all keys (including obsolete ones) since it's populated earlier in buildDirectoryDefinitions (before filtering occurs).

func GenerateIR(yangFiles, includePaths []string, langMapper LangMapper, opts IROptions) (*IR, error) {
  // ...
  mdef, errs := mappedDefinitions(yangFiles, includePaths, opts)
  if errs != nil {
    return nil, errs
  }
  // ...
  directoryMap, errs := buildDirectoryDefinitions(langMapper, mdef.directoryEntries, opts)
  if errs != nil {
    return nil, errs
  }
  // ... 
  dirDets, err := getOrderedDirDetails(langMapper, directoryMap, mdef.schematree, opts)
  if err != nil {
    return nil, util.AppendErr(errs, err)
  }
  // ..
}

This creates a mismatch when writeGoStruct calls generateGetListKey.

func writeGoStruct(targetStruct *ygen.ParsedDirectory, goStructElements map[string]*ygen.ParsedDirectory, generatedUnions map[string]bool, goOpts GoOpts) (GoStructCodeSnippet, []error) {
  // ...

  for _, fName := range targetStruct.OrderedFieldNames() {
    var fieldDef *goStructField

    field := targetStruct.Fields[fName]
    fieldName := goFieldNameMap[fName]
    definedNameMap[fName] = &yangFieldMap{YANGName: fName, GoName: fieldName}
    // ...
  }
  
  // ...
  if err := generateGetListKey(&methodBuf, targetStruct, definedNameMap); err != nil {
    errs = append(errs, err)
  }
  // ...
}

generateGetListKey iterates over all keys in ListKeys, so it tries to access obsolete keys in nameMap, causing nil pointer dereference. nameMap/definedNameMap is built from filtered Fields, so it only contains non-obsolete keys, while ListKeys contains all keys, including obsolete ones.

func generateGetListKey(buf *bytes.Buffer, s *ygen.ParsedDirectory, nameMap map[string]*yangFieldMap) error {
  // ...
  kn := []string{}
  for k := range s.ListKeys {
    kn = append(kn, k)
  }
  sort.Strings(kn)

  for _, k := range kn {
    h.Keys = append(h.Keys, nameMap[k])
  }

  // ...
}

To fix this, I added a safety check in generateGetListKey to skip keys that don't exist in nameMap:

for _, k := range kn {
  // Skip keys that don't exist in nameMap (e.g., when skip_obsolete is enabled)
  if fieldMap, ok := nameMap[k]; ok {
    h.Keys = append(h.Keys, fieldMap)
  }
}

@coveralls
Copy link

Coverage Status

coverage: 88.783% (+0.04%) from 88.744%
when pulling 4fa1ac9 on nleiva:master
into 07135be on openconfig:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants