Removing warnings from libencrypt and libdecrypt #2629

behzadmehmood-rs · 2024-06-25T12:38:16Z

Description

This change simply removes warnings due to type casting.

Related Issue

Motivation and Context

It is necessary for a warning free CI.

How Has This Been Tested?

Types of changes

Bug fix (change which fixes an issue)
New feature (change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My change requires a change to the documentation
I have updated the documentation accordingly
I have added tests to cover my changes
All new and existing tests passed

…verilog-to-routing into rem_warnings

behzadmehmood-rs · 2024-07-09T04:01:37Z

@tangxifan some QoR tests are failing in CI which are not related to the changes in this PR.

tangxifan · 2024-07-09T04:07:09Z

@behzadmehmood-rs Thanks. The CI failures are expected then. Good job.

tangxifan · 2024-07-09T04:08:49Z

@Tulong4Dev Please help on reviewing this PR.

Tulong4Dev

did a quick screening and suggested to make some minor fix.
meanwhile, i'm afraid there could be some memory leakage which i suggest to use valgrind to detect and verify it, if possible.

Tulong4Dev · 2024-07-09T04:50:51Z

libs/libdecrypt/src/decryption.cpp

 * 
- * @param input The base64-encoded input string.
- * @return The decoded output string.
+ * @param encoded he base64-encoded input string.


"he" -> "the"

Tulong4Dev · 2024-07-09T04:58:12Z

libs/libpugiutil/src/pugixml_loc.cpp

-    std::ptrdiff_t offset = 0;
-    char buffer[1024];
-    std::size_t size;
+    //std::ptrdiff_t offset = 0;


remove the commented unused variables.

Tulong4Dev · 2024-07-09T05:22:58Z

libs/libencrypt/src/encryption.cpp

+    unsigned char iv[EVP_MAX_IV_LENGTH];
+    if (RAND_bytes(iv, sizeof(iv)) != 1) {
+        std::cerr << "Error generating IV." << std::endl;
+        return -1;


should return a Boolean here.

Tulong4Dev · 2024-07-09T05:25:55Z

libs/libencrypt/src/encryption.cpp

-    RSA* publicKey = loadPublicKey(publicKeyFile);
+    OpenSSL_add_all_algorithms();
+    ERR_load_crypto_strings();
+    unsigned char sessionKey[16];


can we avoid using constant number here ("16") directly? we can use a meaningful constant variable to represent it.

behzadmehmood · 2024-07-09T06:36:06Z

did a quick screening and suggested to make some minor fix. meanwhile, i'm afraid there could be some memory leakage which i suggest to use valgrind to detect and verify it, if possible.

I have made the changes, now I will run it with valgrind.

behzadmehmood · 2024-07-09T07:16:18Z

did a quick screening and suggested to make some minor fix. meanwhile, i'm afraid there could be some memory leakage which i suggest to use valgrind to detect and verify it, if possible.

I have made the changes, now I will run it with valgrind.

@Tulong4Dev , I have also updated the code to avoid memory leaks.

Tulong4Dev

LGTM. In addition, I'd suggest to add a test with encrypt/decrypt so VTR's Valgrind test flow could help to detect potential memory leak.

behzadmehmood · 2024-07-10T04:19:50Z

LGTM. In addition, I'd suggest to add a test with encrypt/decrypt so VTR's Valgrind test flow could help to detect potential memory leak.

Is it okay to put the test data and code in libdecrypt?

behzadmehmood · 2024-07-10T14:00:34Z

LGTM. In addition, I'd suggest to add a test with encrypt/decrypt so VTR's Valgrind test flow could help to detect potential memory leak.

@Tulong4Dev I have added a test but I could not update workflow file properly. Can you please suggest how should I run the test?

behzadmehmood · 2024-07-15T18:50:47Z

@tangxifan , @Tulong4Dev please review again. Code is updated and test has been added.

tangxifan · 2024-07-21T13:14:21Z

libs/libdecrypt/test/src/test.cpp

+#endif
+
+// Function to create a sample XML file for testing
+void createTestXMLFile(const std::string& filePath) {


@Tulong4Dev Please check if the test provided is sufficient. If a local run on the unit test is o.k., I suggest to merge this PR. This PR is currently a blocking factor on another PR #2637

Removing warnings from libencrypt and libdecrypt

115d237

behzadmehmood-rs requested a review from tangxifan June 25, 2024 12:38

behzadmehmood-rs self-assigned this Jun 25, 2024

github-actions bot added the lang-cpp C/C++ code label Jun 25, 2024

behzadmehmood-rs marked this pull request as draft June 25, 2024 13:07

Updating libencryption/libdecryption for compatibility with openssl3

3eca1f6

github-actions bot added the lang-make CMake/Make code label Jul 8, 2024

behzadmehmood added 2 commits July 8, 2024 17:20

Not dumping decrypted xml

abf1ce6

Commenting unused variables in pugixml_loc.cpp

2f82dd7

github-actions bot added the libpugiutil label Jul 8, 2024

Merge branch 'openfpga' of https://github.com/verilog-to-routing/vtr-…

11e3b5a

…verilog-to-routing into rem_warnings

behzadmehmood-rs marked this pull request as ready for review July 9, 2024 03:59

Tulong4Dev requested changes Jul 9, 2024

View reviewed changes

Minor changes for code clean-up

67fe7a3

Updating code to avoid possible memory leaks

e8b2ac7

Dynamically allocating memory for session key

2482506

Tulong4Dev reviewed Jul 10, 2024

View reviewed changes

Adding test for XML encryption/decryption

7f1df67

github-actions bot added the infra Project Infrastructure label Jul 10, 2024

behzadmehmood added 3 commits July 10, 2024 15:42

Updating CMake for libencrypt

18be453

Correcting typo and removing valgrind call from workflow file

6537c73

Updating build path for Test_Encryption_Decryption

dd5e470

added unit test for libdecryption

6d1dd98

Merge remote-tracking branch 'origin/openfpga' into rem_warnings

deb8802

tangxifan reviewed Jul 21, 2024

View reviewed changes

tangxifan approved these changes Jul 30, 2024

View reviewed changes

Tulong4Dev approved these changes Jul 30, 2024

View reviewed changes

tangxifan merged commit 9eef18c into openfpga Jul 30, 2024
51 of 53 checks passed

tangxifan deleted the rem_warnings branch July 30, 2024 05:01

Removing warnings from libencrypt and libdecrypt #2629

Removing warnings from libencrypt and libdecrypt #2629

Uh oh!

Conversation

behzadmehmood-rs commented Jun 25, 2024

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

behzadmehmood-rs commented Jul 9, 2024

Uh oh!

tangxifan commented Jul 9, 2024

Uh oh!

tangxifan commented Jul 9, 2024

Uh oh!

Tulong4Dev left a comment

Choose a reason for hiding this comment

Uh oh!

Tulong4Dev Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

Tulong4Dev Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

Tulong4Dev Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

Tulong4Dev Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

behzadmehmood commented Jul 9, 2024

Uh oh!

behzadmehmood commented Jul 9, 2024

Uh oh!

Tulong4Dev left a comment

Choose a reason for hiding this comment

Uh oh!

behzadmehmood commented Jul 10, 2024

Uh oh!

behzadmehmood commented Jul 10, 2024

Uh oh!

behzadmehmood commented Jul 15, 2024

Uh oh!

tangxifan Jul 21, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!