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

Jump to bottom

More fixed_width ints and incorporating file_saving_test.c #1025

Merged
merged 1 commit into from Jul 23, 2018
Merged

More fixed_width ints and incorporating file_saving_test.c #1025

merged 1 commit into from Jul 23, 2018

Conversation

hugbubby
Copy link
Member

@hugbubby hugbubby commented Jul 20, 2018 •
edited by iphydf

The file_saving_test.c was not included in the cmake list
and thus was ignored by travis and "make check". I found this
out while introducing ck_assert_msg into the integration test.

Also removed some variable width integers from encryptsave_test.c
and implemented ck_assert_msg, reorganized some loops,
and removed some longs in file_transfer_test.c.

This change is Reviewable

iphydf
iphydf reviewed Jul 20, 2018
View reviewed changes
auto_tests/file_saving_test.c Outdated

size_t written_value = fwrite(cipher, sizeof(*cipher), size, f);
printf("written written_value = %u of %u\n", (unsigned)written_value, (unsigned)size);
printf("written written_value = %u of %u\n", (uint16_t)written_value, (uint16_t)size);
Copy link
Member

Choose a reason for hiding this comment

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

Let's avoid sized-int in printf calls.

Copy link
Member Author

Choose a reason for hiding this comment

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

woops

iphydf
iphydf reviewed Jul 20, 2018
View reviewed changes
auto_tests/encryptsave_test.c Outdated

Suite *encryptsave = encryptsave_suite();
SRunner *test_runner = srunner_create(encryptsave);

int number_failed = 0;
uint8_t number_failed = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just get rid of the SRunner stuff. That's just old compat stuff. Now we just call the test function directly (and we shouldn't use START_TEST anymore, just declare a static function manually).

Copy link
Member Author

Choose a reason for hiding this comment

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

I was never quite sure what SRunner does tbqh. Is my HEAD now equivalent?

@iphydf iphydf modified the milestones: v0.2.4, v0.2.x Jul 20, 2018
@hugbubby hugbubby force-pushed the stdint branch 2 times, most recently from 63f1cad to 3917c41 Compare July 20, 2018 21:58
iphydf
iphydf requested changes Jul 21, 2018
View reviewed changes
auto_tests/encryptsave_test.c Outdated
@@ -43,22 +43,21 @@ static void accept_friend_request(Tox *m, const uint8_t *public_key, const uint8
}
}

START_TEST(test_known_kdf)
static void test_known_kdf()
Copy link
Member

Choose a reason for hiding this comment

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

test_known_kdf(void)

auto_tests/file_saving_test.c Outdated
fseek(f, 0, SEEK_SET);

uint8_t *cipher = (uint8_t *)malloc(size);
uint8_t *clear = (uint8_t *)malloc(size - TOX_PASS_ENCRYPTION_EXTRA_LENGTH);
size_t read_value = fread(cipher, sizeof(*cipher), size, f);
printf("read read_vavue = %u of %ld\n", (unsigned)read_value, size);
printf("Read read_vavue = %u of %ld\n", (uint8_t)read_value, size);
Copy link
Member

Choose a reason for hiding this comment

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

Undo cast.

auto_tests/file_transfer_test.c Outdated
if (!(filename_length == sizeof("Gentoo.exe") && memcmp(filename, "Gentoo.exe", sizeof("Gentoo.exe")) == 0)) {
ck_abort_msg("Bad filename");
}
ck_assert_msg((filename_length == sizeof("Gentoo.exe")
Copy link
Member

Choose a reason for hiding this comment

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

The outer () is now redundant.

auto_tests/file_transfer_test.c Outdated
if (sending_pos != position) {
ck_abort_msg("Bad position %llu", (unsigned long long)position);
}
ck_assert_msg(sending_pos == position, "bad position %llu", (unsigned long long) position);
Copy link
Member

Choose a reason for hiding this comment

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

%llu is not supported on windows. Change to %lu and cast to unsigned long.

auto_tests/file_transfer_test.c Outdated
ck_abort_msg("Could not send chunk, error num=%d pos=%d len=%d", (int)error, (int)position, (int)length);
}
ck_assert_msg(tox_file_send_chunk(tox, friend_number, file_number, position, f_data, length, &error),
"Could not send chunk, error num=%d pos=%d len=%d", (int)error, (int)position, (int)length);
Copy link
Member

Choose a reason for hiding this comment

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

Lowercase "could".

auto_tests/file_transfer_test.c Outdated
++sending_num;
sending_pos += length;

ck_assert_msg(error == TOX_ERR_FILE_SEND_CHUNK_OK, "Wrong error code");
Copy link
Member

Choose a reason for hiding this comment

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

Lowercase "wrong".

auto_tests/file_transfer_test.c Outdated
&& file_accepted == 1,
"Something went wrong in file transfer %u %u %u %u %u %u %lu %lu %lu",
sendf_ok, file_recv, totalf_size == file_size, size_recv == file_size, sending_pos == size_recv,
file_accepted == 1, (unsigned long) totalf_size, (unsigned long) size_recv,
Copy link
Member

Choose a reason for hiding this comment

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

No space after the cast (did you do this or did astyle?).

auto_tests/file_transfer_test.c Outdated
@@ -297,7 +266,7 @@ static void file_transfer_test(void)
(unsigned long)totalf_size, (unsigned long)size_recv,
(unsigned long)sending_pos);

printf("100MiB file sent in %llu seconds\n", time(nullptr) - f_time);
printf("100MiB file sent in %lu seconds\n", time(nullptr) - f_time);
Copy link
Member

Choose a reason for hiding this comment

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

Cast the result of the subtraction to unsigned long.

auto_tests/file_transfer_test.c Outdated
&& sending_pos == size_recv && file_accepted == 1,
"something went wrong in file transfer %u %u %u %u %u %u %u %llu %llu %llu %llu", sendf_ok, file_recv,
m_send_reached, totalf_size == file_size, size_recv == max_sending, sending_pos == size_recv, file_accepted == 1,
(unsigned long long) totalf_size, (unsigned long long) file_size,
Copy link
Member

Choose a reason for hiding this comment

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

No space after cast.

auto_tests/file_transfer_test.c Outdated
@@ -413,3 +372,4 @@ int main(void)
file_transfer_test();
return 0;
}

Copy link
Member

Choose a reason for hiding this comment

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

Remove extra empty line.

@hugbubby hugbubby force-pushed the stdint branch 3 times, most recently from c6adc24 to 6397e17 Compare July 21, 2018 23:23
@hugbubby
Copy link
Member Author

👍?

iphydf
iphydf approved these changes Jul 22, 2018
View reviewed changes
Copy link
Member

@iphydf iphydf left a comment

Choose a reason for hiding this comment

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

:lgtm_strong: nice work, thanks! Only one comment, then it's ready to be merged.

Reviewable status: :shipit: complete! 1 of 1 LGTMs obtained

auto_tests/file_transfer_test.c, line 50 at r2 (raw file):

Previously, iphydf wrote…

The outer () is now redundant.

It's still redundant. I didn't mean the inner (), I meant the outer (). Please put the inner ones back, we currently have that as a standard. We may change that once we use a better code formatter than astyle.

@hugbubby hugbubby force-pushed the stdint branch 6 times, most recently from 7635758 to ffa038f Compare July 23, 2018 06:56
@hugbubby
Copy link
Member Author

dun

@hugbubby @iphydf
More fixed_width ints and incorporating file_saving_test.c
c4d5840 
The file_saving_test.c was not included in the cmake list
and thus was ignored by travis and "make check". I found this
out while introducing ck_assert_msg into the integration test.

Furthermore, removed some variable width integers from encryptsave_test.c,
and the SRunner utilization. Implemmented ck_assert_msg, reorganized some
loops, and removed some longs in file_transfer_test.c.
@iphydf iphydf merged commit c4d5840 into TokTok:master Jul 23, 2018
@iphydf iphydf modified the milestones: v0.2.x, v0.2.5 Aug 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iphydf iphydf iphydf approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.2.5
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@hugbubby @iphydf