Skip to content

Conversation

@Mountain-ql
Copy link

No description provided.

@planetA
Copy link

planetA commented Feb 21, 2023

The PR should follow certain rules. Most importantly a PR must be descriptive. This means that you need to describe a problem you are trying to solve (or feature you are trying to add) and in general terms explain how you solve it. The title of the pull request also must be descriptive.

size_t metrics; // metrics count
double *data; // pointer to the malloc'ed data block, matrix is transposed
struct matrix full; // pointers to the matrix in its original form with all columns
struct matrix full; // pointers to the matrix in its original form with all columns 指向具有所有列的原始形式的矩阵的指针
Copy link

Choose a reason for hiding this comment

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

Do not put comments in Chinese in a repo that is supposed to be widely accessible.

const size_t index_last = llsp->good.columns - 1;

free(llsp->good.matrix[index_last]);
// fprintf(stderr, "good.matrix = %p, index_last = %lu\n", llsp->good.matrix, index_last);
Copy link

Choose a reason for hiding this comment

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

This is a comment left from "printf-debugging" now it serves no purpose. Remove it.

llsp->metrics = count;
llsp->full.columns = count + 1;
llsp->sort.columns = count + 1;
llsp->good.columns = count + 1;
Copy link

Choose a reason for hiding this comment

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

Why is assignment required?

free(llsp->good.matrix[index_last]);
// fprintf(stderr, "good.matrix = %p, index_last = %lu\n", llsp->good.matrix, index_last);
if (llsp->good.matrix) {
free(llsp->good.matrix[index_last]);
Copy link

Choose a reason for hiding this comment

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

Shouldn't there be some loop over all columns? Not just the last one?

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