Skip to content

Implement UniqueID on Darwin#31

Merged
andrewkroh merged 3 commits intoelastic:masterfrom
cwurm:darwin_uniqueid
Dec 21, 2018
Merged

Implement UniqueID on Darwin#31
andrewkroh merged 3 commits intoelastic:masterfrom
cwurm:darwin_uniqueid

Conversation

@cwurm
Copy link
Copy Markdown

@cwurm cwurm commented Dec 20, 2018

Implements #29.

Copy link
Copy Markdown
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding this.

ret, err := C.gethostuuid(&id[0], &wait)
if ret == 0 && err == nil {
bytes := C.GoBytes(unsafe.Pointer(&id[0]), C.int(size))
h.info.UniqueID = hex.EncodeToString(bytes)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the same UUID that I get when I look at "About This Mac" -> "System Report" -> "Hardware UUID" (or ioreg -rd1 -c IOPlatformExpertDevice | grep IOPlatformUUID)?

If so I think people will find it useful if we report the value as a UUID (probably using uuid_unparse) since then they can correlate the ID with asset tracking tools that know the hardware ID.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is. I've added uuid_unparse_upper to have it look exactly the same as it does on macOS.

wait := C.struct_timespec{5, 0} // 5 seconds

ret, err := C.gethostuuid(&id[0], &wait)
if ret == 0 && err == nil {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the time in Go we handle the error case immediately after the function call then return.

if ret != 0 {
  if err != nil {
    // The errno should be set if rtn is non-zero to detail
    // what went wrong (like EWOULDBLOCK).
    return errors.Wrap(err, "gethostuuid failed")
  }
  // It's still an error even if the errno information wasn't set.
  // Note the usage of errors rather than fmt b/c it adds stack
  // context to the error that can be accessed later.
  // https://godoc.org/github.com/pkg/errors#hdr-Formatted_printing_of_errors
  return errors.Errorf("gethostuuid failed")
}

// Now make sense of the uuid_t bits.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@cwurm
Copy link
Copy Markdown
Author

cwurm commented Dec 21, 2018

Even though all three platforms (windows, linux, darwin) have UUIDs as their UniqueIDs they would each be displayed in a different way:

  • Linux is currently all lowercase, no hyphens: 6f7be6fb33e6c67f057266415c084408
  • Windows is currently all lowercase, with hyphens: fa4a0df6-5a59-4870-84a0-cde535805dfc
  • MacOS would be all uppercase, with hyphens: 155320EF-9DB6-5949-966B-F074A556AD32

Would it make sense to unify?

Copy link
Copy Markdown
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Can you please add that one godoc.

Regarding normalizing, I think leaving them as close to their "native" format will be beneficial to users of the library.

"github.com/pkg/errors"
)

func MachineID() (string, error) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add godocs for this that explain that it returns the hardware ID.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Copy Markdown
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andrewkroh andrewkroh merged commit 0a83ff4 into elastic:master Dec 21, 2018
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