Change in flashrom[master]: flashrom.c: do not delete programmer arguments

List overview All Threads
Download

newer

older

Change in ...flashrom[master]:...

Change in flashrom[master]:...

Nikolai Artemiev (Code Review)

19 Aug 2020 19 Aug '20

4:52 a.m.

Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/44562 )

Change subject: flashrom.c: do not delete programmer arguments ......................................................................

flashrom.c: do not delete programmer arguments

Make extract_param() leave parameters in tact rather than deleting them as they are retrieved. Deleting them is undesierable as makes the function more complex and requires that extract_programmer_param() does not get called for the same parameter multiple times, which can be difficult to gaurentee for some programmer initialization cases.

Change-Id: I9b1ec2d273c723fef8908e4ddf13b3079924f0a7 Signed-off-by: Nikolai Artemiev nartemiev@google.com --- M flashrom.c 1 file changed, 0 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/44562/1

diff --git a/flashrom.c b/flashrom.c index 4a30986..f3f0b46 100644 --- a/flashrom.c +++ b/flashrom.c @@ -750,11 +750,6 @@ } strncpy(opt, opt_pos, optlen); opt[optlen] = '\0'; - rest = opt_pos + optlen; - /* Skip all delimiters after the current parameter. */ - rest += strspn(rest, delim); - memmove(param_pos, rest, strlen(rest) + 1); - /* We could shrink haystack, but the effort is not worth it. */ }

return opt;

Attachments:

attachment.htm (text/html — 2.8 KB)

Show replies by thread

Nikolai Artemiev (Code Review)

19 Aug 19 Aug

4:54 a.m.

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/44562

to look at the new patch set (#2).

Change subject: flashrom.c: do not delete programmer arguments ......................................................................

flashrom.c: do not delete programmer arguments

Change-Id: I9b1ec2d273c723fef8908e4ddf13b3079924f0a7 Signed-off-by: Nikolai Artemiev nartemiev@google.com --- M flashrom.c 1 file changed, 2 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/44562/2

Nikolai Artemiev (Code Review)

4:55 a.m.

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/44562

to look at the new patch set (#3).

Change subject: flashrom.c: do not delete programmer arguments ......................................................................

flashrom.c: do not delete programmer arguments

Change-Id: I9b1ec2d273c723fef8908e4ddf13b3079924f0a7 Signed-off-by: Nikolai Artemiev nartemiev@google.com --- M flashrom.c 1 file changed, 3 insertions(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/44562/3

Angel Pons (Code Review)

20 Aug 20 Aug

10:13 a.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44562 )

Change subject: flashrom.c: do not delete programmer arguments ......................................................................

Patch Set 3: Code-Review+1

(3 comments)

https://review.coreboot.org/c/flashrom/+/44562/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/flashrom/+/44562/3//COMMIT_MSG@9 PS3, Line 9: in tact We should also leave the word `intact` intact 😜

https://review.coreboot.org/c/flashrom/+/44562/3//COMMIT_MSG@10 PS3, Line 10: undesierable undesirable

https://review.coreboot.org/c/flashrom/+/44562/3//COMMIT_MSG@13 PS3, Line 13: gaurentee guarantee

-- To view, visit https://review.coreboot.org/c/flashrom/+/44562 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9b1ec2d273c723fef8908e4ddf13b3079924f0a7 Gerrit-Change-Number: 44562 Gerrit-PatchSet: 3 Gerrit-Owner: Nikolai Artemiev nartemiev@google.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Thu, 20 Aug 2020 08:13:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Nikolai Artemiev (Code Review)

11:13 a.m.

Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/44562

to look at the new patch set (#4).

Change subject: flashrom.c: do not delete programmer arguments ......................................................................

flashrom.c: do not delete programmer arguments

Make extract_param() leave parameters intact rather than deleting them as they are retrieved. Deleting them is undesirable as makes the function more complex and requires that extract_programmer_param() does not get called for the same parameter multiple times, which can be difficult to guarantee for some programmer initialization cases.

Change-Id: I9b1ec2d273c723fef8908e4ddf13b3079924f0a7 Signed-off-by: Nikolai Artemiev nartemiev@google.com --- M flashrom.c 1 file changed, 3 insertions(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/44562/4

-- To view, visit https://review.coreboot.org/c/flashrom/+/44562 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9b1ec2d273c723fef8908e4ddf13b3079924f0a7 Gerrit-Change-Number: 44562 Gerrit-PatchSet: 4 Gerrit-Owner: Nikolai Artemiev nartemiev@google.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Nikolai Artemiev (Code Review)

11:14 a.m.

Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44562 )

Change subject: flashrom.c: do not delete programmer arguments ......................................................................

Patch Set 4:

(3 comments)

https://review.coreboot.org/c/flashrom/+/44562/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/flashrom/+/44562/3//COMMIT_MSG@9 PS3, Line 9: in tact

...

We should also leave the word `intact` intact 😜

lol, done.

https://review.coreboot.org/c/flashrom/+/44562/3//COMMIT_MSG@10 PS3, Line 10: undesierable

...

undesirable

Done

https://review.coreboot.org/c/flashrom/+/44562/3//COMMIT_MSG@13 PS3, Line 13: gaurentee

...

guarantee

Done

-- To view, visit https://review.coreboot.org/c/flashrom/+/44562 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9b1ec2d273c723fef8908e4ddf13b3079924f0a7 Gerrit-Change-Number: 44562 Gerrit-PatchSet: 4 Gerrit-Owner: Nikolai Artemiev nartemiev@google.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Thu, 20 Aug 2020 09:14:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Angel Pons (Code Review)

11:17 a.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44562 )

Change subject: flashrom.c: do not delete programmer arguments ......................................................................

Patch Set 4: Code-Review+2

-- To view, visit https://review.coreboot.org/c/flashrom/+/44562 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9b1ec2d273c723fef8908e4ddf13b3079924f0a7 Gerrit-Change-Number: 44562 Gerrit-PatchSet: 4 Gerrit-Owner: Nikolai Artemiev nartemiev@google.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Thu, 20 Aug 2020 09:17:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Nico Huber (Code Review)

1:54 p.m.

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44562 )

Change subject: flashrom.c: do not delete programmer arguments ......................................................................

Patch Set 4: Code-Review-1

I understand why it is unwanted for your case, but I don't understand why the code is suddenly not needed anymore.

-- To view, visit https://review.coreboot.org/c/flashrom/+/44562 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9b1ec2d273c723fef8908e4ddf13b3079924f0a7 Gerrit-Change-Number: 44562 Gerrit-PatchSet: 4 Gerrit-Owner: Nikolai Artemiev nartemiev@google.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Thu, 20 Aug 2020 11:54:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Angel Pons (Code Review)

6:40 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44562 )

Change subject: flashrom.c: do not delete programmer arguments ......................................................................

Patch Set 4: Code-Review+1

-- To view, visit https://review.coreboot.org/c/flashrom/+/44562 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9b1ec2d273c723fef8908e4ddf13b3079924f0a7 Gerrit-Change-Number: 44562 Gerrit-PatchSet: 4 Gerrit-Owner: Nikolai Artemiev nartemiev@google.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Thu, 20 Aug 2020 16:40:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Nikolai Artemiev (Code Review)

25 Aug 25 Aug

4:17 a.m.

Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44562 )

Change subject: flashrom.c: do not delete programmer arguments ......................................................................

Patch Set 4:

...

Patch Set 4: Code-Review-1

I understand why it is unwanted for your case, but I don't understand why the code is suddenly not needed anymore.

My bad here, I didn't realize that deleting parameters is necessary to allow unused parameters to be detected. I'll look into adapting our current code paths that call extract_param() multiple times.

-- To view, visit https://review.coreboot.org/c/flashrom/+/44562 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9b1ec2d273c723fef8908e4ddf13b3079924f0a7 Gerrit-Change-Number: 44562 Gerrit-PatchSet: 4 Gerrit-Owner: Nikolai Artemiev nartemiev@google.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Tue, 25 Aug 2020 02:17:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Nikolai Artemiev (Code Review)

4:18 a.m.

Nikolai Artemiev has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/44562 )

Change subject: flashrom.c: do not delete programmer arguments ......................................................................

Abandoned

Change is not compatible with unused parameter detection.

-- To view, visit https://review.coreboot.org/c/flashrom/+/44562 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9b1ec2d273c723fef8908e4ddf13b3079924f0a7 Gerrit-Change-Number: 44562 Gerrit-PatchSet: 4 Gerrit-Owner: Nikolai Artemiev nartemiev@google.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: abandon

1339

days inactive

1345

days old

flashrom-gerrit@flashrom.org

10 comments

3 participants

tags (0)

participants (3)

Angel Pons (Code Review)
Nico Huber (Code Review)
Nikolai Artemiev (Code Review)