Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Fix improper retain level on return in [UIFont initWithCoder]#1263

Merged
ms-jihua merged 3 commits into
microsoft:developfrom
ms-jihua:check_overrelease
Oct 28, 2016
Merged

Fix improper retain level on return in [UIFont initWithCoder]#1263
ms-jihua merged 3 commits into
microsoft:developfrom
ms-jihua:check_overrelease

Conversation

@ms-jihua

@ms-jihua ms-jihua commented Oct 28, 2016

Copy link
Copy Markdown
Contributor

Fixes #1260


This change is Reviewable

@msft-Jeyaram

msft-Jeyaram commented Oct 28, 2016

Copy link
Copy Markdown
}

fix this up too?

This looks horribly bad. #Resolved


Refers to: Frameworks/UIKit/UIFont.mm:311 in 99920d9. [](commit_id = 99920d9, deletion_comment = False)

@msft-Jeyaram

Copy link
Copy Markdown
}

cleaned up and follow a proper format?
It has a weird pattern of self assignment and pretty much using the objects static method to generate an object within the shell object.
I'm sure it can be cleaned up using CTFontCreateWithName directly or a wrap around it (if we want to keep encapsulation)


In reply to: 257024231 [](ancestors = 257024231)


Refers to: Frameworks/UIKit/UIFont.mm:311 in 99920d9. [](commit_id = 99920d9, deletion_comment = False)

@msft-Jeyaram

msft-Jeyaram commented Oct 28, 2016

Copy link
Copy Markdown

:shipit:

@DHowett-MSFT

Copy link
Copy Markdown

:shipit:

CGFloat size = [coder decodeFloatForKey:@"UIFontPointSize"];

return reinterpret_cast<UIFontPrototype*>(static_cast<UIFont*>(CTFontCreateWithName((__bridge CFStringRef)name, size, nullptr)));
}

@bbowman bbowman Oct 28, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this not valuable for derived? (if thats a thing) UIFonts. I'm just slightly confused as initWithCoder isn't a typical designated initializer is it? Is there something we should be providing on the base class that calls a different designated initializer? #ByDesign

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  • re: valuable for derived: this is a WinObjC-only extension, and we don't have any UIFont derived classes, nor should we ever.
  • re: designated initializers: UIFont has no initializers we could route to :(

In reply to: 85619643 [](ancestors = 85619643)

Comment thread Frameworks/UIKit/UICTFont.mm Outdated
// WinObjC-only extension for UINibUnarchiver
- (instancetype)initWithCoder:(NSCoder*)coder {
NSString* name = [coder decodeObjectForKey:@"UIFontName"];
if ([name length] < 1) {

@bbowman bbowman Oct 28, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[name length] < 1 [](start = 8, length = 17)

length < 1 seems like a curious way of writing this. #Resolved

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah, now that you mention it (this was copied forward from some really old code). I'll change it to == 0.


In reply to: 85619699 [](ancestors = 85619699)

@bbowman

bbowman commented Oct 28, 2016

Copy link
Copy Markdown
Member

:shipit:

@ms-jihua ms-jihua merged commit fcf430d into microsoft:develop Oct 28, 2016
@ms-jihua ms-jihua deleted the check_overrelease branch February 1, 2017 22:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants