Skip to content

Commit

Permalink
Add a #ifdef to silence a clang static analyzer false positive wa…
Browse files Browse the repository at this point in the history 
…rning.

The current (at the time of this writing) version of the `clang` static analyzer is complaing that the `objects` pointer is `NULL`.  This is a false positive warning from the analyzer.
  • Loading branch information
@johnezang
johnezang committed Jul 12, 2012
1 parent 5663f2d commit 8215763
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions JSONKit.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,9 @@ - (void)getObjects:(id *)objectsPtr range:(NSRange)range
NSParameterAssert((objects != NULL) && (count <= capacity));
if((objectsPtr == NULL) && (NSMaxRange(range) > 0UL)) { [NSException raise:NSRangeException format:@"*** -[%@ %@]: pointer to objects array is NULL but range length is %lu", NSStringFromClass([self class]), NSStringFromSelector(_cmd), (unsigned long)NSMaxRange(range)]; }
if((range.location > count) || (NSMaxRange(range) > count)) { [NSException raise:NSRangeException format:@"*** -[%@ %@]: index (%lu) beyond bounds (%lu)", NSStringFromClass([self class]), NSStringFromSelector(_cmd), (unsigned long)NSMaxRange(range), (unsigned long)count]; }
#ifndef __clang_analyzer__
memcpy(objectsPtr, objects + range.location, range.length * sizeof(id));
#endif
}

- (id)objectAtIndex:(NSUInteger)objectIndex
Expand Down

15 comments on commit 8215763

@Daij-Djan
Copy link

Choose a reason for hiding this comment

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

im afraid I see it as true.. the objectPtr could be NULL and it would crash.
objectPtr = NULL
range = NSMakeRange(1,1)

@Bo98
Copy link

@Bo98 Bo98 commented on 8215763 Jan 11, 2013

Choose a reason for hiding this comment

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

See #84 (comment)

@schubter
Copy link

Choose a reason for hiding this comment

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

@Daij-Djan, you could write it as this:
if ((true) && (true)) { raise }

as objectsPtr == NULL will be true
and NSMaxRange of range 1,1 will also be greater then 0

seems like the compiler does not notice throwing an exception will stop the execution right there

@Daij-Djan
Copy link

Choose a reason for hiding this comment

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

@schubter:: yes sorry blush I meant a zero range! (e.g. 1,0) => the idea is that the if doesn't have to trigger ....

@guykogus
Copy link

Choose a reason for hiding this comment

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

@wwwang89 This is not the place to ask such questions. GitHub is for reporting issues and requesting features. If you want to ask technical questions like how to download files from GitHub, either read the git/GitHub documentation or ask this question on a forum such as StackOverflow.

@Daij-Djan
Copy link

@Daij-Djan Daij-Djan commented on 8215763 Mar 6, 2013 via email

Choose a reason for hiding this comment

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

@basvk
Copy link

@basvk basvk commented on 8215763 Mar 22, 2013

Choose a reason for hiding this comment

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

One way to solve this without silencing the analyzer would be to add this at the top of the method:
if (range.length == 0) return;

No need to start copying anything anyway when the range length is 0.

This avoids a crash when calling [getObjects:NULL range:NSMakeRange(0, 0)], which is why the analyzer was complaining in the first place (so no false positive).

@poorbird
Copy link

Choose a reason for hiding this comment

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

JSONKit will crash in the ios9, 'NSTaggedPointString', i can't resolve it

@guykogus
Copy link

Choose a reason for hiding this comment

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

This may not be a useful answer, but why are you still using JSONKit? NSJSONSerialization has been available since iOS 5. There are no iOS devices you should be supporting that can't use it. This library hasn't been update since 2012. It's time to let it go.

@poorbird
Copy link

Choose a reason for hiding this comment

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

@guykogus I know, but my team started use in early, so.. need some time.
now, i still resolve it...

@guykogus
Copy link

Choose a reason for hiding this comment

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

Can you share the JSON string that you're trying to parse?

@poorbird
Copy link

Choose a reason for hiding this comment

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

@guykogus after iOS9.0 ，like '0.4' , system change it become NSTaggedPointString, change '12345678' become CFString, NSTaggedPointString string will crash in the JSONKit, ‘- (NSString *)JSONString’ method.
Do you understand.

@guykogus
Copy link

Choose a reason for hiding this comment

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

No... please provide the JSON string and code you're using.

@poorbird
Copy link

Choose a reason for hiding this comment

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

@guykogus ， resolve it just now, #185

@jcbertin
Copy link

Choose a reason for hiding this comment

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

I have a fork that handle tagged pointers the right way : see #158.

Please sign in to comment.