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.

Sign up for GitHub

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

fix: Avoided dereferencing NULL pointer #34836

Merged
merged 1 commit into from
Jul 27, 2024
Merged

fix: Avoided dereferencing NULL pointer #34836

merged 1 commit into from
Jul 27, 2024

Conversation

Ald392
Copy link
Contributor

@Ald392 Ald392 commented Jul 19, 2024

issue : #34835

@sre-ci-robot sre-ci-robot added the size/S Denotes a PR that changes 10-29 lines. label Jul 19, 2024
@sre-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Ald392
To complete the pull request process, please assign wxyucs after the PR has been reviewed.
You can assign the PR to them by writing /assign @wxyucs in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mergify mergify bot added dco-passed DCO check passed. kind/bug Issues or changes related a bug labels Jul 19, 2024
@Ald392
Copy link
Contributor Author

Ald392 commented Jul 23, 2024

/retest

@sre-ci-robot
Copy link
Contributor

@Ald392: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@Ald392 Ald392 force-pushed the nullptrfix branch 2 times, most recently from cb7a5cd to c5d4962 Compare July 23, 2024 06:00
longjiquan
longjiquan suggested changes Jul 23, 2024
View reviewed changes
internal/core/src/indexbuilder/init_c.cpp Outdated
@@ -23,6 +24,9 @@ char*
IndexBuilderSetSimdType(const char* value) {
auto real_type = milvus::config::KnowhereSetSimdType(value);
char* ret = reinterpret_cast<char*>(malloc(real_type.length() + 1));
if (ret == nullptr) {
throw std::runtime_error("Error: Memory allocation failed");
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to use "AssertInfo" instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. Currently working on it.

@longjiquan
Copy link
Contributor

/ok-to-test

@mergify mergify bot added the ci-passed label Jul 23, 2024
@codecov Codecov
Copy link

codecov bot commented Jul 23, 2024 •
edited

Codecov Report

Attention: Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 80.96%. Comparing base (a4d7da4) to head (64bf73f).
Report is 6 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #34836    +/-   ##
========================================
  Coverage   80.95%   80.96%            
========================================
  Files        1185     1180     -5     
  Lines      142758   143433   +675     
========================================
+ Hits       115569   116129   +560     
- Misses      22782    22883   +101     
- Partials     4407     4421    +14
Files Coverage Δ
internal/core/src/segcore/segcore_init_c.cpp 41.02% <100.00%> (-2.57%) ⬇️
internal/core/src/indexbuilder/init_c.cpp 0.00% <0.00%> (ø)
internal/core/src/log/Log.cpp 0.00% <0.00%> (ø)
internal/core/src/segcore/metrics_c.cpp 0.00% <0.00%> (ø)

... and 246 files with indirect coverage changes

@sre-ci-robot sre-ci-robot added size/XS Denotes a PR that changes 0-9 lines. and removed size/S Denotes a PR that changes 10-29 lines. labels Jul 23, 2024
@mergify mergify bot removed the ci-passed label Jul 23, 2024
@Ald392
Copy link
Contributor Author

Ald392 commented Jul 23, 2024

/retest

@Ald392 Ald392 force-pushed the nullptrfix branch 2 times, most recently from 47a8075 to cc76e5e Compare July 23, 2024 18:19
@mergify Mergify
Copy link
Contributor

mergify bot commented Jul 23, 2024

@Ald392 E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

longjiquan
longjiquan reviewed Jul 24, 2024
View reviewed changes
Copy link
Contributor

@longjiquan longjiquan left a comment

Choose a reason for hiding this comment

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

/lgtm

@longjiquan
Copy link
Contributor

/run-cpu-e2e

@longjiquan
Copy link
Contributor

rerun ut

@sre-ci-robot sre-ci-robot removed the lgtm label Jul 24, 2024
@sre-ci-robot sre-ci-robot added size/S Denotes a PR that changes 10-29 lines. and removed size/XS Denotes a PR that changes 0-9 lines. labels Jul 24, 2024
@Ald392
Copy link
Contributor Author

Ald392 commented Jul 24, 2024

/retest

@Ald392
fix: Avoided dereferencing NULL pointer
64bf73f 
issue : milvus-io#34835

Signed-off-by: Ald392 <imagesai32@gmail.com>
@mergify mergify bot added the ci-passed label Jul 24, 2024
@Ald392
Copy link
Contributor Author

Ald392 commented Jul 24, 2024 •
edited

@longjiquan The Mac code checker is failing continually for all other PRs as well and it needs a fix. So can you provide any suggestion on how we should proceed ?

@Ald392 Ald392 mentioned this pull request Jul 26, 2024
Closed
1 task
@xiaofan-luan
Copy link
Contributor

/lgtm
/approve

@xiaofan-luan xiaofan-luan merged commit 9463eee into milvus-io:master Jul 27, 2024
9 of 12 checks passed
sumitd2 pushed a commit to sumitd2/milvus that referenced this pull request Aug 6, 2024
@Ald392 @sumitd2
fix: Avoided dereferencing NULL pointer (milvus-io#34836)
72054cb 
issue : milvus-io#34835

Signed-off-by: Ald392 <imagesai32@gmail.com>
Signed-off-by: Sumit Dubey <sumit.dubey2@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@longjiquan longjiquan longjiquan requested changes

@godchen0212 godchen0212 Awaiting requested review from godchen0212

@xiaocai2333 xiaocai2333 Awaiting requested review from xiaocai2333

Assignees

@longjiquan longjiquan

@xiaofan-luan xiaofan-luan

Labels
ci-passed dco-passed DCO check passed. kind/bug Issues or changes related a bug lgtm ok-to-test size/S Denotes a PR that changes 10-29 lines.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Ald392 @sre-ci-robot @longjiquan @xiaofan-luan