Skip to content

Conversation

@cjwind
Copy link
Contributor

@cjwind cjwind commented Feb 23, 2019

do_remove_head() checked removes[string_length + 1] only, so
q_remove_head() would pass the test when it try to write data to other
place in the buffer, such as index string_length + 2. Check entire
buffer to fix it.

do_remove_head() checked removes[string_length + 1] only, so
q_remove_head() would pass the test when it try to write data to other
place in the buffer, such as index string_length + 2. Check entire
buffer to fix it.
qtest.c Outdated
}

bool check_overflow = true;
int i = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the scope of int i into for-loop. So, it would look like the following:

for (int i = string_length + 1; i < string_length + STRINGPAD; i++) {
    ...

qtest.c Outdated

bool check_overflow = true;
int i = 0;
for (i = string_length + 1; i < string_length + STRINGPAD; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the algorithm descriptions about checking and validating memory here.

} else if (removes[string_length + 1] != 'X') {
}

/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tweak the indention for comments.

Something like:

/*
  How large is a queue before it's considered big.
  This affects how it gets printed
  and whether cautious mode is used when freeing the list
*/

qtest.c Outdated
Validate memory by checking removes's padding positions are still
initial value 'X'.
*/
bool check_overflow = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder the necessity of variable check_overflow. Can you eliminate it by replacing for with while?

Variable check_overflow isn't necessary.
qtest.c Outdated
break;
}
int i = string_length + 1;
while (i < string_length + STRINGPAD && removes[i] == 'X') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add parentheses. So, it becomes while ((i < string_length + STRINGPAD) && removes[i] == 'X') {

qtest.c Outdated
}

/*
Validate memory by checking removes's padding positions are still
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check for grammar within the comment.

@jserv jserv merged commit a36453a into sysprog21:master Feb 24, 2019
threesmaller pushed a commit to threesmaller/lab0-c that referenced this pull request Feb 24, 2019
do_remove_head() checked removes[string_length + 1] only, so
q_remove_head() would unexpectedly pass the test when it attempts
to write data to other place in the buffer, such as
index string_length + 2. Check entire buffer to fix it.
tfliao added a commit to tfliao/lab0-c that referenced this pull request Feb 28, 2019
    - Test sysprog21#7 still failed, leave it for investigate
    - Match swp file in .gitignore
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 this pull request may close these issues.

2 participants