Date
1 - 11 of 11
string pointer
|
Gerriet M. Denkmann
I have a method like (macOS 13.3, Xcode 9.2 (9C40b)):
- (NSUInteger)computeFor: (NSUInteger)arg reason: (NSString * __autoreleasing *)reason { NSUInteger value; // some computing… if ( reason != NULL ) *reason = @“Used Chebycheff approximation”; return value; } to be use like: NSUInteger a = [ self computeFor: 12 reason: NULL ]; // don’t care for reason or: NSString *s NSUInteger b = [ self computeFor: 99 reason: &s ]; // want to know reason I am not quite sure whether __autoreleasing is the correct annotation. Is it? But my real problem is this: BOOL needReason = … NSString *s; NSString **stringPointer = needReason ? &s : NULL; NSUInteger c = [ self computeFor: 42 reason: stringPointer ]; // sometimes want to know the reason I cannot figure out how to declare the stringPointer without getting compiler warnings. How to do this? Gerriet |
|
|
|
Steve Mills
On Mar 1, 2018, at 06:13:19, Gerriet M. Denkmann <g@...> wrote:
I am not quite sure whether __autoreleasing is the correct annotation. Is it?Yes. But my real problem is this:Why not this? NSString* s; NSUInteger c = [self computeFor:42 reason:needReason ? &s : nil]; -- Steve Mills Drummer, Mac geek |
|
|
|
Gerriet M. Denkmann
Why not this?Excellent. Works perfectly without complaints from the compiler. Thanks a lot! Kind regards, Gerriet. |
|
|
|
Quincey Morris
On Mar 1, 2018, at 04:13 , Gerriet M. Denkmann <g@...> wrote:
So, here’s what’s going on. It’s a bit messy. — A pointer has an ownership attribute, which is __strong by default, but can be explicitly specified as __weak or __autoreleasing. In your code, variable “s” is __strong by default. — A pointer to a pointer needs to know the ownership of the pointee, and there is no default (in most cases), so you get the error "Pointer to non-const type 'NSString *' with no explicit ownership” for your declaration of “stringPointer”. — If you specify the actual ownership explicitly: NSString *__strong*stringPointer = needReason ? &s : NULL; that error goes away, but there is still an error on the call: “Passing address of non-local object to __autoreleasing parameter for write-back”. That’s because a pointer to a pointer that’s a method parameter *does* have a default pointee ownership, “__autoreleasing”. (When you added the explicit “__autoreleasing”, you didn’t change anything that wasn’t already assumed.) — So, you can declare your “computeFor:” method like this: - (NSUInteger)computeFor: (NSUInteger)arg reason: (NSString *__strong*)reason and everything would be just great, and this is the safest route. The only irritation is that every "NSString**” you pass into “computeFor:reason:” is going to need to be explicitly annotated, including those that pass through other methods, so the need to annotate is going to ripple outwards across your code. — This solution: NSUInteger c = [self computeFor:42 reason:needReason ? &s : nil]; is AFAIK a bit dangerous. The “computeFor:reason:” method stores an *autoreleased* string pointer into its output parameter, so “s” will end up containing this autoreleased pointer when “needReason” is true. That autoreleased pointer is a ticking time bomb that will explode if it’s still being used somewhere when its autorelease pool is drained (which could be a *lot* later, leading to hard-to-find bug) without being owned somewhere else. Whether this is a problem depends on what you do with “s” next. FWIW, all APIs that have a "NSError**outError” parameter have this same danger. (Charles Srstka pointed this out a few months ago.) Our apps work because the unowned pointer usually gets thrown away fast, or gets stored somewhere that gives it an extra retain. |
|
|
|
Steve Mills
On Mar 1, 2018, at 13:05:37, Quincey Morris <quinceymorris@...> wrote:I disagree that it's dangerous. It's the standard way code is written when a parameter is passed back by reference. How many times in Apple code do you see something like: NSError* err; if([thing doStuffAndReturnError:&err]) ; else ; // Handle error. They don't add any decoration to the local variable err. And every time you autocomplete a method body that includes a reference param, Xcode includes __autoreleasing between the splats: -(BOOL) doStuffAndReturnError:(NSError*__autoreleasing*)err; They even say this in the "Transitioning to ARC Release Notes": • __autoreleasing is used to denote arguments that are passed by reference (id *) and are autoreleased on return. Then goes on to explain how the compiler takes care of things by rewriting it for you. -- Steve Mills Drummer, Mac geek |
|
|
|
Quincey Morris
On Mar 1, 2018, at 11:18 , Steve Mills <sjmills@...> wrote:
(I tried to find Charles’s post about this, but didn’t succeed.) Everything you said after the first sentence is absolutely true. That is the standard way, and your suggestion helped rewrite the code according to the standard way without a lot of fuss. But it’s still dangerous. You can Try It at Home™. Put this code in a new app’s delegate: - (void) computeReason: (NSObject **) reason Run it and it works. Now comment out the 2 commented lines. Run it and it crashes. That’s because “s” contains an autoreleased pointer. It’s fine so long as nothing drains the autorelease pool, but if something does, it fails. The fact that “s” is supposed to be __strong is (unfortunately) not respected by the called method, which thinks it’s __autoreleasing. Normally, it’s not a problem, because the drain of the autorelease pool is up at the main event loop, long after these methods have returned. However, if the pointer ends up in a different pool (as in this contrived example) with a shorter lifetime, that’s bad. This example is not so very contrived, though. It isn’t uncommon to use a local autorelease pool for compute-bound loops, and if any errors happen during the loop the app can crash. Also, if you send pointers off to GCD queues, the autorelease pool lifetimes are hard to predict. BTW, the original code would *never* fail, for an unrelated reason. The pointer in that case was to a literal NSString, which isn’t reference counted, and is alive forever. |
|
|
|
Dave
On 1 Mar 2018, at 19:18, Steve Mills <sjmills@...> wrote:Just because you see it in “Apple Code” doesn’t mean its the best or safest way of doing something. Just look at the quality of code coming out of Apple these days, its awful, in fact just look at the mess that is called XCode!On Mar 1, 2018, at 13:05:37, Quincey Morris <quinceymorris@...> wrote:I disagree that it's dangerous. It's the standard way code is written when a parameter is passed back by reference. How many times in Apple code do you see something like: Autoreleasing bugs are really difficult to find which is why I avoid using AU like the plague! All the Best Dave |
|
|
Without having used any apps you've written, I can't compare. And 'out' parameters are widely used in Cocoa programming, not just by Apple.
I honestly don't see anything dangerous about the code in question. And your "ticking time bomb" explanation makes no sense to me (and I've been using Cocoa since 2000.) There's no difference between an autoreleased pointer that you get as the return value of a function, vs. one that comes from an 'out' parameter like this. —Jens |
|
|
|
Dave
The difference is in the amount of resources available to each of us. If you were to take into account he reliability of my software given the amount of resources I have available then I’d score over Apple on a scale of at least 1000%
I’m not complaining about “out” parameters, just autorelease!
Its been ages since I’ve had to deal with it and I am talking pre ARC which takes care of 99% of the worry of using Auto Release. But the problem occurred when you assigned the object returned in that manner to a property or iVar and didn’t retain it. If you accessed before had been released by AU, it worked ok, but if it was a while later (usually in response to a notification) it causes a crash. At that point its hard to know what the cause of crash is whereas if it is released manually you’d get the crash near the point of the bug a very short time afterwards. I think the NSError class is more prone to these crashes because it typically isn’t accessed a lot after the event. Cheers Dave
|
|
|
That's [was] true of any value. Whether it's a method return value or an 'out' parameter makes no difference. If you didn't retain it, you didn't own it, and it might go away. So this really has nothing to do with 'out' parameters, and it's not an issue nowadays because of ARC. —Jens |
|
|
|
Dave
Yes, it has to do with autorelease, but there are usually options for getting a non-autoreleased version, e.g. usually an init method to create it, however you are (were) stuck with an released version is passed back in an out statement, which is what I meant by happening mostly on NSError objects.
toggle quoted message
Show quoted text
|
|
|