Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2877 +/- ##
==========================================
+ Coverage 78.19% 78.20% +0.01%
==========================================
Files 685 687 +2
Lines 118074 118305 +231
Branches 16617 16652 +35
==========================================
+ Hits 92330 92525 +195
- Misses 24856 24894 +38
+ Partials 888 886 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I believe the Valgrind failure is this one: https://github.com/aws/aws-lc/actions/runs/20031393257/job/57441202249?pr=2877#step:4:1663 |
| return false; | ||
| } | ||
|
|
||
| BIGNUM *raw = NULL; |
There was a problem hiding this comment.
| BIGNUM *raw = NULL; | |
| bssl::UniquePtr<BIGNUM> raw; |
There was a problem hiding this comment.
BN_hex2bn requires BIGNUM **outp as a parameter so we can't use bssl::UniquePtr<BIGNUM> here :(
| #include "../tool/internal.h" | ||
| #include "internal.h" | ||
|
|
||
| #define BUF_SIZE 1024 |
There was a problem hiding this comment.
We should abstract BUF_SIZE from req.cc to a shared header.
Line 31 in ec39cb3
Same for the other PR as well.
There was a problem hiding this comment.
I feel like the choice of buffer size among different commands can be a little arbitrary. Standardizing that across all commands might not be a good idea?
| break; | ||
| } | ||
|
|
||
| inlen = fread(inbuf, 1, sizeof(inbuf), in_file.get()); |
There was a problem hiding this comment.
Can we use ReadFromFD here? And use ScopedFD for all the file descripters
There was a problem hiding this comment.
Can we create a ScopedFD for stdin? My main reason for using ScopedFILE was because I couldn't find a straightforward way to use ScopedFD for stdin
### Description of changes: This PR addresses the remaining comments from PRs #2877 and #2898. I also took the liberty to clean up `tool-openssl` to make our code easier to read. ### Testing: Unit tests By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license. --------- Co-authored-by: Samuel Chiang <sachiang@amazon.com>
### Description of changes: This PR addresses the remaining comments from PRs aws#2877 and aws#2898. I also took the liberty to clean up `tool-openssl` to make our code easier to read. ### Testing: Unit tests By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license. --------- Co-authored-by: Samuel Chiang <sachiang@amazon.com>
Description of changes:
Implement enc CLI with the following options:
Testing:
Unit tests
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.