Skip to content

Conversation

derickr
Copy link
Contributor

@derickr derickr commented Jan 2, 2018

No description provided.

@derickr derickr requested a review from jmikola January 2, 2018 13:48
Copy link
Member

Choose a reason for hiding this comment

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

Why was this removed? All of our get_properties_hash functions have such a check to return props early if the object is not initialized. Binary has:

if (!intern->data) {
	return props;
}

For structs that don't have a null pointer, we have an initialized property, as with UTCDateTime:

if (!intern->initialized) {
	return props;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As this if() statement always evaluated to false (as intern->oid was always true), this actually never happened, and it didn't fail any tests...

I hadn't realise we did this for stuff that doesn't have a null pointer though, so will need to change that, but the question is really: how do we test this?

Copy link
Contributor Author

@derickr derickr Jan 3, 2018

Choose a reason for hiding this comment

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

I've pushed a new commit to put back intern->initialized 🇺🇸 (should really be initialised 🇬🇧).

Copy link
Member

Choose a reason for hiding this comment

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

(should really be initialised 🇬🇧)

That's poppycock. See: https://stackoverflow.com/questions/157807/gb-english-or-us-english (this answer in particular)

@derickr derickr force-pushed the PHPC-1049-build-warning branch from 1525074 to 5a1dba9 Compare January 3, 2018 11:47
Copy link
Member

Choose a reason for hiding this comment

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

Good catch. I forgot that oid was a statically allocated char array. That might explain why the warning only complained about ObjectId and not the char * fields in other structs.

@derickr derickr force-pushed the PHPC-1049-build-warning branch from 5a1dba9 to eb8d067 Compare January 4, 2018 10:40
@derickr derickr merged commit eb8d067 into mongodb:master Jan 4, 2018
derickr added a commit that referenced this pull request Jan 4, 2018
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