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

minor bug in awk statement in zpool_id script. #259

Closed
aarcane opened this issue May 31, 2011 · 9 comments
Closed

minor bug in awk statement in zpool_id script. #259

aarcane opened this issue May 31, 2011 · 9 comments
Milestone

Comments

@aarcane
Copy link

aarcane commented May 31, 2011

https://github.com/behlendorf/zfs/blob/master/cmd/zpool_id/zpool_id#L52 the < and > in the awk line result in an inability to parse the zdev.conf file unless the path id is wrapped in < and > like a1 pci-0000:00:1f.2-scsi-0:0:0:0 .

@aarcane
Copy link
Author

aarcane commented May 31, 2011

I got another link, aparently it's an awk vs. awk issue with ubuntu dependancies.

the following alternate may be better:

ID_ZPOOL=$( grep -w ${ID_PATH} ${CONFIG} | sed -e "s/\s${ID_PATH}//" )

it doesn't depend on awk and all linuces include grep and sed standard.

@aarcane
Copy link
Author

aarcane commented May 31, 2011

and a minor update, -w doesn't work like it's supposed to, but this does:

ID_ZPOOL=$( grep "\s${ID_PATH}$" ${CONFIG} | sed -e "s/\s${ID_PATH}//" )

fully tested with duplicate lines and working.

@dajhorn
Copy link
Contributor

dajhorn commented May 31, 2011

The \s character class seems to be a PCRE extension in GNU sed. (See the sed/testsuite/PCRE.tests file.)

I tried this in a busybox rescue environment, but the expression was unrecognized. A conservative [ ]+ is more likely to be reliable on all systems.

@behlendorf
Copy link
Contributor

I remember hitting this once before, at the time it looked to me like an issue with mawk which is the default awk on Ubuntu. When I installed gawk it was able to match the pattern correctly. I'm all for doing something more portable if possible but the regex rule may be a little tricker that it first appears. It must do the following:

  • Ignore all lines which begin with a comment
  • Return on the first match
  • Match the empty string and the beginning and end

@aarcane
Copy link
Author

aarcane commented May 31, 2011

On 5/31/2011 09:31, behlendorf wrote:

I remember hitting this once before, at the time it looked to me like an issue with mawk which is the default awk on Ubuntu. When I installed gawk it was able to match the pattern correctly. I'm all for doing something more portable if possible but the regex rule may be a little tricker that it first appears. It must do the following:

  • Ignore all line which begin with a comment
  • Return on the first match
  • Match the empty string and the beginning and end

I think a better idea for now is to simply add gawk to the list of
requirements for ubuntu releases.. or perhaps updating the format for
the file to include some sort of control characters around the text <
and > work, except can you match those with all awks ?

Perhaps we should use it as a shell file instead? I'm not sure if
arrays work in sh though.. it's a tough position. does the gawk
statement work in a busybox environment ?

@aarcane
Copy link
Author

aarcane commented Jun 1, 2011

On 5/31/2011 08:10, dajhorn wrote:

The \s character class seems to be a PCRE extension in GNU sed. (See the sed/testsuite/PCRE.tests file.)

I tried this in a busybox rescue environment, but the expression was unrecognized. A conservative [ ]+ is more likely to be reliable on all systems.

[ ]+ is not going to work. [ \t] might work better, because \s matches
all space characters, which includes nbsps, spaces, tabs, and vtabs at
least.

@ulope
Copy link

ulope commented Dec 30, 2011

This bit me (again) today on a new machine with Ubuntu 11.10. It would be great to have this fixed.

Maybe as a workaround until a portable solution is found you could add gawk as a dependency to the ppa?

@dajhorn
Copy link
Contributor

dajhorn commented Dec 30, 2011

@ulope: Can you post a zdev.conf file that causes the problem? -- Either to the zfs-discuss list or a Github gist.

I will try fixing the bug generally before adding a dependency to the PPA.

behlendorf pushed a commit that referenced this issue Jan 11, 2012
Some implementations of `awk` incorrectly parse the \< and \> regex
symbols, so use a `while read` loop and regular globbing instead.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes: #259
@dajhorn
Copy link
Contributor

dajhorn commented Jan 24, 2012

@behlendorf or @aarcane: You can close this milestone issue. It looks like Github doesn't automatically act on the commit "Closes:" line if it comes after an attribution line.

Rudd-O pushed a commit to Rudd-O/zfs that referenced this issue Feb 1, 2012
Some implementations of `awk` incorrectly parse the \< and \> regex
symbols, so use a `while read` loop and regular globbing instead.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes: openzfs#259
fuhrmannb pushed a commit to fuhrmannb/cstor that referenced this issue Nov 3, 2020
…#259)

Signed-off-by: nsathyaseelan <sathyaseelan.n@mayadata.io>
andrewc12 added a commit to andrewc12/openzfs that referenced this issue Aug 7, 2023
Signed-off-by: Andrew Innes <andrew.c12@gmail.com>
EchterAgo pushed a commit to EchterAgo/zfs that referenced this issue Sep 21, 2023
Signed-off-by: Andrew Innes <andrew.c12@gmail.com>
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 a pull request may close this issue.

4 participants