jaypipes / pcidb

Small golang library for querying PCI database (pciids) information
Apache License 2.0
34 stars 16 forks source link

Provide the option to provide a path directly to the pci-id file (both #18

Closed brendank310 closed 3 years ago

brendank310 commented 5 years ago

compressed or uncompressed) and the option to disable reaching out to an Internet URL for fetching.

Signed-off-by: Brendan Kerrigan kerriganb@ainfosec.com

brendank310 commented 5 years ago

Hi Brendan,

Thanks very much for your PR! My first reaction is that I think this is trying to introduce multiple unrelated things in the same patch.

Yeah, apologies for lumping both things into the one PR.

For example, you could submit a separate PR that just introduces a new DisableNetworkFetch (or similar) configuration option that only served to disable the attempt to retrieve a PCI-IDS file from the Internet.

Perhaps instead of DisableNetworkFetch, an option to specify the URL would be more flexible? With an empty URL indicating no fetching.

A second PR could introduce a SearchPaths configuration option that allowed the user to override the search paths if needed. Though, I'm not entirely sure if a SearchPaths configuration option is really necessary -- after all, for special environments that needed some custom PCI-IDS file location, you could simply set the CachePath configuration variable and write a PCI-IDS file to that location...

That sounds like a better named option. The environment I'm using this in has a read-only rootfs generated with OpenEmbedded (that ships the compressed pci-ids file), and the home directory is not persistent.

Third, instead of a separate foundCompressedPath variable in the load() method, you could simply add to the searchPaths configuration option (on Linux) additional file paths that end in .gz and .tgz and then instead of doing this:

f, err := os.Open(foundPath)
...
scanner := bufio.NewScanner(f)

you could do something like this:

var r io.Reader
var err error
f, err := os.Open(foundPath)
...
if strings.HasSuffix(foundPath, "gz") {
        r, err = gzip.NewReader(f)
  if err != nil {
      log.Fatal(err)
  }
} else {
        r, err = bufio.NewReader(f)
  if err != nil {
      log.Fatal(err)
  }
}
scanner := bufio.NewScanner(r)
return parseDBFile(db, scanner)

Looks reasonable to me, just came across your project yesterday and have been integrating with it today. This and ghw have proven very useful, thanks!

That way, there would be no need to change the cacheDBFile() function at all.

Lemme know if these suggestions/requests make sense or if you have any questions for me.

All the best, -jay