Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WARNING strconv.ParseInt on kernel with LOCALVERSION set #3628

Closed
rla opened this issue Jan 16, 2014 · 9 comments
Closed

WARNING strconv.ParseInt on kernel with LOCALVERSION set #3628

rla opened this issue Jan 16, 2014 · 9 comments

Comments

@rla
Copy link

rla commented Jan 16, 2014

So far this has been reported and fixed for the special cases only:

Today I ran (0.7.5) on a kernel with not LOCALVERSION separator (with uname 3.12.8tag) where tag is arbitrary non-integer word. This will give warning:

strconv.ParseInt: parsing "8tag": invalid syntax

There is nothing in the kernel compilation process that requires LOCALVERSION to be prefixed/separated with . or - (a good convention but not always done).

@tianon
Copy link
Member

tianon commented Jan 16, 2014

I've tagged this as helpwanted - it should be a reasonably easy fix if someone wants to take a stab at it. It would be an excellent first foray into Go, too. Just be sure to comment here if you're planning to work on it and we'll be happy to help with any issues you might run into!

@rla
Copy link
Author

rla commented Jan 17, 2014

Indeed, I'm not a Go programmer (yet) but I do not fear Go or coding. I haven't had time to look yet where is the flavour used (seems that's what it's called in the code) and what could be possible backwards-compatibility issues. Current cases must stay but should we also allow additional separators (+ and _?) to . and - or just take plain string when those two cases above do not match.

@tianon
Copy link
Member

tianon commented Jan 20, 2014

I think it's only used for feature/bug compatibility comparisons, so stripping any non-numeric prefix should be appropriate behavior.

@chazomaticus
Copy link
Contributor

Hope I didn't step on your toes, @rla -- I've been looking to get my feet wet in docker and real-world go, and this seemed like an easy fix, so I just went for it.

@tianon, the easiest thing to do seemed to be just ignoring the kernel "flavor" entirely. It didn't seem to be used anywhere, although I haven't been using docker for very long so perhaps I missed something.

@tianon
Copy link
Member

tianon commented Jan 21, 2014

It's included in the output of docker info, where it's turned out to be useful every now and then for determining the specific "kernel flavor" (ie, for kernels compiled with the gentoo patchset, we typically get -gentoo as the flavor).

@chazomaticus
Copy link
Contributor

Gotcha. I'll define "flavor" as anything that comes after the minor version number in the string and print it verbatim.

I'll also use Sscanf like you mention in the PR, 'cause that'll be way easier!

@rla
Copy link
Author

rla commented Jan 21, 2014

@chazomaticus, no problems. I'm happy that this gets fixed!

@chazomaticus
Copy link
Contributor

I believe this is fixed from #3695. Not sure if it's still open for any reason other than that I forgot to include the magic "fixes #..." text in the PR.

@tianon
Copy link
Member

tianon commented Jan 30, 2014

Indeed, thanks!

@tianon tianon closed this as completed Jan 30, 2014
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

No branches or pull requests

3 participants