fix compiler warnings in SessionManager.mm

RESOLVED FIXED

Status

Camino Graveyard
General
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Sean Murphy, Assigned: Sean Murphy)

Tracking

({fixed1.8.1.3})

Trunk
PowerPC
Mac OS X
fixed1.8.1.3

Details

Attachments

(1 attachment)

(Assignee)

Description

11 years ago
Created attachment 256530 [details] [diff] [review]
patch

I noticed two minor problems with the session manager code that we might want to straighten up for the sake of correctness as well as preventing compiler warnings.  Neither were affecting functionality or caused any major problem.

> SessionManager.mm:133: warning: `BookmarkToolbar' may not respond to `-isVisible'
> SessionManager.mm:180: warning: `BookmarkToolbar' may not respond to `-setVisible:'

#import "BookmarkToolbar.h".

> SessionManager.mm:141: warning: invalid conversion from `const NSString*' to `NSString*'
> [... 14 more ...]

The constant NSString keys used throughout SessionManager.mm are currently declared like this:
const NSString* kBookmarkBarIsVisibleKey = @"BookmarkBarVisible";

Which mandates that the object itself (NSString/NSConstantString) must remain constant, while the pointer to that object (kBookmarkBarIsVisibleKey) is permitted to change and refer to another address.  NSStrings/NSContstantStrings are already immutable by definition.

Instead, marking the pointer variable itself as constant results in the correct behavior:
static NSString* const kBookmarkBarIsVisibleKey = @"BookmarkBarVisible";

This will prevent those conversions from occurring.  Also, the prior declaration did not impose any restriction on reassigning a key to another string, whereas this approach will cause the compiler to complain.  Adding the static keyword keeps the keys local to this file only.
Attachment #256530 - Flags: review?

Comment 1

11 years ago
Comment on attachment 256530 [details] [diff] [review]
patch

Whoops, that was a braindead mistake. I thought I ran that code through a warning check, but apparently not.

r/sr=smorgan
(Although I really need to rework session saving to be more like serialization, so SM isn't so dependent on knowing about all these classes)
Attachment #256530 - Flags: review? → superreview+

Comment 2

11 years ago
Checked in on trunk and MOZILLA_1_8_BRANCH.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.1.3
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.