[gtk] random string cleanup

RESOLVED WONTFIX

Status

Core Graveyard
GFX: Gtk
--
minor
RESOLVED WONTFIX
15 years ago
9 years ago

People

(Reporter: dwitte@gmail.com, Assigned: dwitte@gmail.com)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

34.43 KB, patch
Brian Ryner (not reading)
: review-
dwitte@gmail.com
: superreview+
Details | Diff | Splinter Review
(Assignee)

Description

15 years ago
during alecf's recent charset whackage, I noticed some odd string fu in
widget/src/gtk. he promised to sr a little string cleanup at a later date ;)
(Assignee)

Comment 1

15 years ago
Created attachment 126986 [details] [diff] [review]
patch v1
(Assignee)

Comment 2

15 years ago
Comment on attachment 126986 [details] [diff] [review]
patch v1

just a little string cleanup - feel free to take your time with it. thx!
Attachment #126986 - Flags: superreview?(alecf)
Attachment #126986 - Flags: review?(blizzard)
*** Bug 211544 has been marked as a duplicate of this bug. ***

Comment 4

15 years ago
Comment on attachment 126986 [details] [diff] [review]
patch v1

 nsClipboard::nsClipboard()
+ : mIgnoreEmptyNotification(PR_FALSE)
 {
 #ifdef DEBUG_CLIPBOARD
   g_print("nsClipboard::nsClipboard()\n");
 #endif /* DEBUG_CLIPBOARD */

-  mIgnoreEmptyNotification = PR_FALSE;
-  mGlobalTransferable = nsnull;
-  mSelectionTransferable = nsnull;
-  mGlobalOwner = nsnull;
-  mSelectionOwner = nsnull;
-  mSelectionData.data = nsnull;
   mSelectionData.length = 0;

we dont' need to init these variables? (just checking..)
sr=alecf with an explanation or the variables initialized

nice cleanup!
Attachment #126986 - Flags: superreview?(alecf) → superreview+
(Assignee)

Comment 5

15 years ago
good catch - looks like I was a bit overzealous... the transferables/owners are
COMPtrs so they don't need initing, but mSelectionData.data does. new patch
coming up...
(Assignee)

Updated

15 years ago
Attachment #126986 - Flags: review?(blizzard)
(Assignee)

Comment 6

15 years ago
Created attachment 127766 [details] [diff] [review]
patch v1.1

carrying over sr=alecf.
Attachment #126986 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #127766 - Flags: superreview+
Attachment #127766 - Flags: review?(blizzard)
(Assignee)

Comment 7

14 years ago
Comment on attachment 127766 [details] [diff] [review]
patch v1.1

trying a different reviewer.
Attachment #127766 - Flags: review?(blizzard) → review?(bryner)
Comment on attachment 127766 [details] [diff] [review]
patch v1.1

>Index: widget/src/gtk/nsClipboard.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/widget/src/gtk/nsClipboard.cpp,v
>retrieving revision 1.112
>diff -u -1 -0 -p -r1.112 nsClipboard.cpp
>--- widget/src/gtk/nsClipboard.cpp	11 Jun 2003 18:15:33 -0000	1.112
>+++ widget/src/gtk/nsClipboard.cpp	15 Jul 2003 01:11:13 -0000
>@@ -535,28 +519,28 @@ nsClipboard::SelectionReceiver (GtkWidge
> #endif
>     mSelectionData.length = aSD->length;
>     return;
>   }
> 
>   char *str = gdk_atom_name(aSD->type);
>   nsCAutoString type(str);
>   g_free(str);
> 
> #ifdef DEBUG_CLIPBOARD
>-  g_print("        Type is %s\n", type.mBuffer);
>+  g_print("        Type is %s\n", type.get());
> 
>-  if (type.Equals("ATOM")) {
>+  if (type.Equals(NS_LITERAL_CSTRING("ATOM"))) {

Is there an advantage to this type of change? (you're doing it in several
places, and IMO it makes the code less readable)

>@@ -1276,58 +1239,40 @@ void nsClipboard::SetCutBuffer()
>     nsPrimitiveHelpers::ConvertUnicodeToPlatformPlainText (castedUnicode, dataLength / 2,
>                                                            &plainTextData, &plainTextLen);
>     if (clipboardData) {
>       nsMemory::Free(NS_REINTERPRET_CAST(char*, clipboardData));
>       clipboardData = plainTextData;
>       dataLength = plainTextLen;
>     }
>   }
> 
>   XRotateBuffers(GDK_DISPLAY(), 1);
>-  XStoreBytes(GDK_DISPLAY(), (const char *) clipboardData, nsCRT::strlen((const char *)clipboardData));
>+  XStoreBytes(GDK_DISPLAY(), (const char *) clipboardData, PL_strlen((const char *)clipboardData));

I think we can just use strlen() here.
Comment on attachment 127766 [details] [diff] [review]
patch v1.1

marking review- to get this out of my queue... feel free to re-request review
if you've added explanations for the questions I had and still feel that this
patch is good.
Attachment #127766 - Flags: review?(bryner) → review-
widget/src/gtk/ has been removed on trunk.
This bug doesn't look like a branch candidate to me.

-> WONTFIX
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → WONTFIX
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.