Skip to content

Commit 4dc879e

Browse files
committed
Variable name, comment, and string changes based on PR review
1 parent 64ab766 commit 4dc879e

File tree

5 files changed

+34
-29
lines changed

5 files changed

+34
-29
lines changed

.github/workflows/examples-esp32.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ jobs:
121121
name: ESP32_1
122122

123123
runs-on: ubuntu-latest
124-
if: github.actor != 'restyled-io[bot]' && github.repository_owner == 'espressif'
124+
if: github.actor != 'restyled-io[bot]'
125125

126126
container:
127127
image: ghcr.io/project-chip/chip-build-esp32:94

.github/workflows/qemu.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ jobs:
3838
BUILD_TYPE: esp32-qemu
3939

4040
runs-on: ubuntu-latest
41-
if: github.actor != 'restyled-io[bot]' && github.repository_owner == 'espressif'
41+
if: github.actor != 'restyled-io[bot]'
4242

4343
container:
4444
image: ghcr.io/project-chip/chip-build-esp32-qemu:94

.vscode/tasks.json

+3-3
Original file line numberDiff line numberDiff line change
@@ -104,19 +104,19 @@
104104
}
105105
},
106106
{
107-
"label": "Build ESP32 Unit Tests (QEMU)",
107+
"label": "Build esp32-qemu unit tests",
108108
"type": "shell",
109109
"command": "source scripts/activate.sh; src/test_driver/esp32/clean.sh; scripts/build/build_examples.py --target esp32-qemu-tests build",
110110
"problemMatcher": []
111111
},
112112
{
113-
"label": "Clean ESP32 Unit Tests (QEMU)",
113+
"label": "Clean esp32-qemu unit tests",
114114
"type": "shell",
115115
"command": "src/test_driver/esp32/clean.sh",
116116
"problemMatcher": []
117117
},
118118
{
119-
"label": "Run ESP32 Unit Tests (QEMU)",
119+
"label": "Run esp32-qemu unit tests",
120120
"type": "shell",
121121
"command": "src/test_driver/esp32/run_qemu_image.py --verbose --file-image-list ./out/esp32-qemu-tests/test_images.txt",
122122
"problemMatcher": []

src/lib/support/CHIPArgParser.cpp

+28-23
Original file line numberDiff line numberDiff line change
@@ -331,18 +331,23 @@ bool ParseArgs(const char * progName, int argc, char * const argv[], OptionSet *
331331
int currentElement = 0;
332332
int currentCharacter;
333333
// The value of optind after the last time getopt_long was called.
334-
int prevOptind = 1;
334+
int prevOptIndex = 1;
335335
// All the nonoptions we have found so far will be in range [firstNonoption, lastNonoptionPlus1).
336336
int firstNonoption = 1;
337337
int lastNonoptionPlus1 = 1;
338+
338339
// Temporary variables used When finding a new set of nonoptions.
339340
int firstNonoptionNew;
340341
int lastNonoptionPlus1New;
341-
// Temporary variable used when mergeing existing nonoptions with new ones.
342+
343+
// Temporary variables used when moving already-found nonoptions later in the array so that they
344+
// will be contiguous with the new non-options that were just found.
345+
// Index the nonoption is being moved to.
342346
int moveNonoptionToHere;
343-
// Index variables used in permuting argv.
344-
int idx;
345-
int idx2;
347+
// Initial index of the nonoption being moved.
348+
int nonoptionToMove;
349+
// Index of the next pair of elements to swap in argv (indexToSwap and indexToSwap+1).
350+
int indexToSwap;
346351
// Temporary variable for swapping elements of argv.
347352
char * tempSwap;
348353
// Permutable version of argv. Needed to move the nonoptions to the end.
@@ -449,23 +454,23 @@ bool ParseArgs(const char * progName, int argc, char * const argv[], OptionSet *
449454
// elements of argv during the execution of this loop may not always match what they would be with the POSIX version. It is
450455
// only guaranteed to match at the end.
451456

452-
// The elements that just now got processed by getopt_long are [prevOptind, optind] if the new option is the non-final
453-
// character of an option group (e.g. 'a' or 'b' in "-abc"), or [prevOptind, optind) if it's the final character of an
457+
// The elements that just now got processed by getopt_long are [prevOptIndex, optind] if the new option is the non-final
458+
// character of an option group (e.g. 'a' or 'b' in "-abc"), or [prevOptIndex, optind) if it's the final character of an
454459
// option group or not a group at all. Walk through those elements knowing that all elements we encounter before reaching
455460
// the option must be nonoptions. Store the range of those nonoptions in firstNonoptionNew and lastNonoptionPlus1New. If
456-
// this is the final iteration (getopt_long returned -1) then prevOptind == optind == argc, which means firstNonoptionNew ==
461+
// this is the final iteration (getopt_long returned -1) then prevOptIndex == optind == argc, which means firstNonoptionNew ==
457462
// lastNonoptionPlus1New == argc, the following for loop will be skipped, and all the nonoptions we had already found will
458463
// get moved to the end of argv.
459-
firstNonoptionNew = prevOptind;
460-
lastNonoptionPlus1New = prevOptind;
461-
for (idx = prevOptind; idx <= optind; idx++)
464+
firstNonoptionNew = prevOptIndex;
465+
lastNonoptionPlus1New = prevOptIndex;
466+
for (nonoptionToMove = prevOptIndex; nonoptionToMove <= optind; nonoptionToMove++)
462467
{
463-
// Note that this loop INCLUDES idx=optind since the option that just got processed might be part of a group of short
468+
// Note that this loop INCLUDES nonoptionToMove=optind since the option that just got processed might be part of a group of short
464469
// options all sharing a single '-', in which case optind is going to land on that same element several times in a row
465470
// before it moves past it.
466-
if (argv[idx] && argv[idx][0] == '-')
471+
if (argv[nonoptionToMove] && argv[nonoptionToMove][0] == '-')
467472
{
468-
lastNonoptionPlus1New = idx;
473+
lastNonoptionPlus1New = nonoptionToMove;
469474
break;
470475
}
471476
}
@@ -476,15 +481,15 @@ bool ParseArgs(const char * progName, int argc, char * const argv[], OptionSet *
476481
// Move the old set of nonoptions we had already found so that they abut the new set of nonoptions we just now found,
477482
// thus giving us a single contiguous block of nonoptions.
478483
moveNonoptionToHere = firstNonoptionNew - 1;
479-
for (idx = lastNonoptionPlus1 - 1; idx >= firstNonoption; idx--)
484+
for (nonoptionToMove = lastNonoptionPlus1 - 1; nonoptionToMove >= firstNonoption; nonoptionToMove--)
480485
{
481-
// Move element idx (the last nonoption we haven't moved yet) into the slot moveNonoptionToHere.
482-
for (idx2 = idx; idx2 < moveNonoptionToHere; idx2++)
486+
// Move element nonoptionToMove (the last nonoption we haven't moved yet) into the slot moveNonoptionToHere.
487+
for (indexToSwap = nonoptionToMove; indexToSwap < moveNonoptionToHere; indexToSwap++)
483488
{
484-
// Swap element idx2 with element idx2 + 1.
485-
tempSwap = permutableArgv[idx2];
486-
permutableArgv[idx2] = permutableArgv[idx2 + 1];
487-
permutableArgv[idx2 + 1] = tempSwap;
489+
// Swap element indexToSwap with element indexToSwap + 1.
490+
tempSwap = permutableArgv[indexToSwap];
491+
permutableArgv[indexToSwap] = permutableArgv[indexToSwap + 1];
492+
permutableArgv[indexToSwap + 1] = tempSwap;
488493
}
489494
// Decrement moveNonoptionToHere so the next nonoption we move will go into the slot before it.
490495
moveNonoptionToHere--;
@@ -495,7 +500,7 @@ bool ParseArgs(const char * progName, int argc, char * const argv[], OptionSet *
495500
}
496501

497502
// Store the value of optind so we'll know which elements getopt_long processes the next time it is called.
498-
prevOptind = optind;
503+
prevOptIndex = optind;
499504

500505
#endif // CONFIG_NON_POSIX_GETOPT_LONG
501506

@@ -588,7 +593,7 @@ bool ParseArgs(const char * progName, int argc, char * const argv[], OptionSet *
588593
if (nonOptArgHandler != nullptr)
589594
{
590595
#ifdef CONFIG_NON_POSIX_GETOPT_LONG
591-
// On a POSIX implementation, on the final interation when getopt_long returns -1 indicating it has nothing left to do,
596+
// On a POSIX implementation, on the final iteration when getopt_long returns -1 indicating it has nothing left to do,
592597
// optind would be set to the location of the first nonoption (all of which by now would have been moved to the end of
593598
// argv). On some non-POSIX implementations this is not true -- it simply sets optind to the location of argv's terminal
594599
// NULL (i.e. optind == argc). So we have to alter optind here to simulate the POSIX behavior.

src/test_driver/esp32/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ To build all unit tests:
5858
$ source scripts/activate.sh
5959
$ scripts/build/build_examples.py --target esp32-qemu-tests build
6060

61-
This generates a list of QEMU images in `out/esp32-qemu-tests/`
61+
This generates a set of QEMU images in `out/esp32-qemu-tests/`
6262

6363
There is one image for each test directory (i.e. each chip_test_suite). So for
6464
example `src/inet/tests` builds to `out/esp32-qemu-tests/testInetLayer.img`

0 commit comments

Comments
 (0)