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

cpiofs #11

Merged
merged 1 commit into from Nov 26, 2019
Merged

cpiofs #11

merged 1 commit into from Nov 26, 2019

Conversation

@crmulliner
Copy link
Collaborator

crmulliner commented Nov 25, 2019

  • add cpiofs (driver to treat cpio files as a filesystem)
@crmulliner crmulliner requested review from Grazfather and jlarimer Nov 25, 2019
@crmulliner crmulliner force-pushed the collin/cpiofs branch from 2786fd5 to 774018f Nov 25, 2019
}

func normalizePath(filepath string) (dir string, name string) {
// Ensure directory and file names are consistent, with no relative parts

This comment has been minimized.

Copy link
@Grazfather

Grazfather Nov 25, 2019

Collaborator

Put this as a doc string above the func so we get it in godoc.

reg := p.fileInfoReg
nameIdx := 7
dirpath := ""
if line[0] == 'b' || line[0] == 'c' {

This comment has been minimized.

Copy link
@Grazfather

Grazfather Nov 25, 2019

Collaborator

string.HasPrefix


lines := strings.Split(string(out), "\n")
for _, line := range lines {
if len(line) < 11 {

This comment has been minimized.

Copy link
@Grazfather

Grazfather Nov 25, 2019

Collaborator

Where does the 11 here and the 10 above come from?


func (p *CpioParser) Supported() bool {
_, err := exec.LookPath(cpioCmd)
if err != nil {

This comment has been minimized.

Copy link
@Grazfather

Grazfather Nov 25, 2019

Collaborator

might as well combine

if _, err := ...; err != nil {
if err != nil {
return false
}
_, err = exec.LookPath(cpCmd)

This comment has been minimized.

Copy link
@Grazfather

Grazfather Nov 25, 2019

Collaborator

this is fine but i think it would be easier to read this function if it matched the above

if _, err := ...; err != nil {
    return true
}
return false
LinkTarget string
}

func TestMain(t *testing.T) {

This comment has been minimized.

Copy link
@Grazfather

Grazfather Nov 25, 2019

Collaborator

Can we come up with better names?

}
}

func TestMain2(t *testing.T) {

This comment has been minimized.

Copy link
@Grazfather

Grazfather Nov 25, 2019

Collaborator

Better name

if _, err := os.Stat(testImage); err != nil {
t.Error(err)
} else {
os.Remove(testfilename)

This comment has been minimized.

Copy link
@Grazfather

Grazfather Nov 25, 2019

Collaborator

we should probably remove this in a test fixture. If the above test fails because the test file exists, it'll keep failing until cleared manually.

@crmulliner crmulliner force-pushed the collin/cpiofs branch from 774018f to 3286235 Nov 26, 2019
@Grazfather

This comment has been minimized.

Copy link
Collaborator

Grazfather commented Nov 26, 2019

Looks good. I still don't know where the idx = 7 comes from and the len == 10. Could we add a few consts for things like that?

Collin Mulliner
@crmulliner crmulliner force-pushed the collin/cpiofs branch from 3286235 to 04e669e Nov 26, 2019
@crmulliner crmulliner merged commit b27199e into master Nov 26, 2019
2 checks passed
2 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.