[haiku-commits] Re: r37670 - in haiku/trunk: build/jam headers/os/drivers headers/private/graphics/matrox headers/private/graphics/neomagic src/add-ons/accelerants/ati ...

On Thu, Jul 22, 2010 at 9:36 AM, Axel Dörfler <axeld@xxxxxxxxxxxxxxxx> wrote:
> zooey@xxxxxxxxxxxxxxx wrote:
>> Log:
>> * applied patch by kaliber that fixes more than 100 warnings - thanks
>> a lot!
>> Closes #6349
>
> There are lots of questionable fixes, though. Smaller patches would
> have been appreciated, too. Anyway, thanks for the work!

Thanks for review! Attached patch hopefully fixes my questionable changes.

>> +++ haiku/trunk/headers/os/drivers/config_manager.h   2010-07-21 21:
>> 43:20 UTC (rev 37670)
>> @@ -106,6 +106,8 @@
>>                                               uint32 len);
>>  } config_manager_for_driver_module_info;
>>
>> +int config_manager_scan_hardcoded(struct device_info **info, int32
>> *count);
>
> Must not be in here, and cannot be used by users of that module anyway.

Is config_manager_scan_hardcoded() really used? I didn't find a
reference to this function.

>
>>                       OUTREGM(R128_CRTC_EXT_CNTL,
>> -                             R128_CRTC_DISPLAY_DIS | R128_CRTC_HSYNC_DIS, 
>> mask);
>> +                             (R128_CRTC_DISPLAY_DIS | R128_CRTC_HSYNC_DIS), 
>> mask);
>
> As François already pointed out, the macro should be fixed instead.

Fixed.

>> +++ haiku/trunk/src/add-ons/accelerants/ati/rage128_init.cpp  2010-07-
>> 21 21:43:20 UTC (rev 37670)
>> @@ -21,10 +21,10 @@
>>  // Memory Specifications from RAGE 128 Software Development Manual
>>  // (Technical Reference Manual P/N SDK-G04000 Rev 0.01), page 3-21.
>>  static R128_RAMSpec sRAMSpecs[] = {
>> -     { 4, 4, 3, 3, 1, 3, 1, 16, 12, "128-bit SDR SGRAM 1:1" },
>> -     { 4, 8, 3, 3, 1, 3, 1, 17, 13, "64-bit SDR SGRAM 1:1" },
>> -     { 4, 4, 1, 2, 1, 2, 1, 16, 12, "64-bit SDR SGRAM 2:1" },
>> -     { 4, 4, 3, 3, 2, 3, 1, 16, 12, "64-bit DDR SGRAM" },
>> +     { 4, 4, 3, 3, 1, 3, 1, 16, 12, (char *)"128-bit SDR SGRAM 1:1" },
>> +     { 4, 8, 3, 3, 1, 3, 1, 17, 13, (char *)"64-bit SDR SGRAM 1:1" },
>> +     { 4, 4, 1, 2, 1, 2, 1, 16, 12, (char *)"64-bit SDR SGRAM 2:1" },
>> +     { 4, 4, 3, 3, 2, 3, 1, 16, 12, (char *)"64-bit DDR SGRAM" },
>
> The structure should be made const char* instead.

Fixed.

>> +++ haiku/trunk/src/add-ons/kernel/bus_managers/firewire/firewire.c
>> 2010-07-21 21:43:20 UTC (rev 37670)
>> @@ -1096,7 +1096,6 @@
>>  fw_tl2xfer(struct firewire_comm *fc, int node, int tlabel, int tcode)
>>  {
>>       struct fw_xfer *xfer;
>> -     int s = splfw();
>>       int req;
>
> I looked it up specifically, and splfw() doesn't have any function
> anymore.

I wonder, if splx(s) should be called instead of remove splfw.

>> +++ haiku/trunk/src/add-ons/kernel/drivers/graphics/common/log_dump.h
>> 2010-07-21 21:43:20 UTC (rev 37670)
>> @@ -13,5 +13,6 @@
>>  #include <SupportDefs.h>
>>
>>  void log_printall( FILE *logfile, char *buffer, uint32 buffer_len );
>> +void log_printentry( FILE *logfile, log_entry *entry );
>
> This one doesn't belong here. It's not a function that could or should
> be called from the outside (log_entry is not even known outside the
> source file).

Fixed.

> ons/mail_daemon/inbound_filters/spam_filter/SpamFilterConfig.cpp        2010-
> 07-21 21:43:20 UTC (rev 37670)
>> @@ -209,7 +209,7 @@
>>
>>       tempRect.right = fNoWordsMeansSpamCheckBoxPntr->Frame().left -
>>               be_plain_font->StringWidth ("a");
>> -     tempStringPntr = "Spam above:";
>> +     tempStringPntr = (char *)"Spam above:";
>>       sprintf (numberString, "%06.4f", (double) fSpamCutoffRatio);
>>       fSpamCutoffRatioTextBoxPntr     = new BTextControl (
>>               tempRect,
>> @@ -228,7 +228,7 @@
>>
>>       // Add the box displaying the genuine cutoff ratio, on a line by
>> itself.
>>
>> -     tempStringPntr = "Genuine below and uncertain above:";
>> +     tempStringPntr = (char *)"Genuine below and uncertain above:";
>
> Why not make it const?

Fixed.

>> +++ haiku/trunk/src/add-ons/media/media-add-ons/dvb/DVBMediaNode.cpp
>> 2010-07-21 21:43:20 UTC (rev 37670)
>> @@ -1099,6 +1099,7 @@
>>               fSelectedState, fSelectedRegion, fSelectedChannel,
>> fSelectedAudio);
>>
>>       status_t err;
>> +     bool needs_tuning = false;
>>
>>       if (fSelectedChannel < 0 || fSelectedAudio < 0) {
>>               printf("DVBMediaNode::Tune: invalid tuning info\n");
>> @@ -1135,7 +1136,6 @@
>>               return B_OK;
>>       }
>>  */
>> -     bool needs_tuning;
>>       switch (fInterfaceType) {
>>               case DVB_TYPE_DVB_T:
>>                       needs_tuning = (fTuningParam.u.dvb_t.frequency !=
>> new_params.u.dvb_t.frequency) || !fTuningSuccess;
>
> I can't think of a reason why this should be moved upwards that much.

GCC2 complains:
src/add-ons/media/media-add-ons/dvb/DVBMediaNode.cpp: In method
`status_t DVBMediaNode::Tune()':
src/add-ons/media/media-add-ons/dvb/DVBMediaNode.cpp:1192: jump to label `end'
src/add-ons/media/media-add-ons/dvb/DVBMediaNode.cpp:1107:   from here
src/add-ons/media/media-add-ons/dvb/DVBMediaNode.cpp:1138:   crosses
initialization of `bool needs_tuning'

>> +++ haiku/trunk/src/add-ons/media/media-add-
>> ons/firewire_dv/FireWireCard.cpp      2010-07-21 21:43:20 UTC (rev 37670)
>> @@ -75,7 +75,7 @@
>>       {203, 2997},    /* = (8000 - 29.97 * 250)/(29.97 * 250) */
>>       {1, 15},        /* = (8000 - 25 * 300)/(25 * 300) */
>>  };
>> -char *system_name[] = {"NTSC", "PAL"};
>> +char *system_name[] = {(char *)"NTSC", (char *)"PAL"};
>
> Rather make it const.

Fixed.

>> +++ haiku/trunk/src/add-ons/media/media-add-
>> ons/opensound/OpenSoundAddOn.cpp      2010-07-21 21:43:20 UTC (rev 37670)
>> @@ -64,7 +64,7 @@
>>       fInitCheckStatus = B_NO_INIT;
>>
>>       /* unix paths */
>> -     if (RecursiveScan("/dev/oss/") != B_OK)
>> +     if (RecursiveScan((char *)"/dev/oss/") != B_OK)
>
> This had been nicely fixed in the other MultiAudioAddOn.h file, why
> this brute force change here?

Fixed.

>> [... truncated: 584 lines follow ...]
>
> Those only have minor issues :-)

Also fixed ;-)

-- 
Grzegorz Dąbrowski (kaliber)
http://tiltos.com
http://home.gna.org/pingwinek/
Index: src/apps/soundrecorder/TrackSlider.cpp
===================================================================
--- src/apps/soundrecorder/TrackSlider.cpp      (revision 37696)
+++ src/apps/soundrecorder/TrackSlider.cpp      (working copy)
@@ -75,7 +75,7 @@
        
        fBitmap = new BBitmap(rect, BScreen().ColorSpace(), true);
        
-       fBitmapView = new SliderOffscreenView(rect.OffsetToSelf(B_ORIGIN), 
(char *)"bitmapView");
+       fBitmapView = new SliderOffscreenView(rect.OffsetToSelf(B_ORIGIN), 
"bitmapView");
        fBitmap->AddChild(fBitmapView);
        
        fBitmapView->fRight = Bounds().right - kLeftRightTrackSliderWidth;
@@ -477,7 +477,7 @@
 }
 
 
-SliderOffscreenView::SliderOffscreenView(BRect frame, char *name)
+SliderOffscreenView::SliderOffscreenView(BRect frame, const char *name)
        : BView(frame, name, B_FOLLOW_LEFT | B_FOLLOW_TOP, B_WILL_DRAW),
        leftBitmap(BRect(BPoint(0,0), kLeftRightTrackSliderSize), B_CMAP8),
        rightBitmap(BRect(BPoint(0,0), kLeftRightTrackSliderSize), B_CMAP8),
Index: src/apps/soundrecorder/TrackSlider.h
===================================================================
--- src/apps/soundrecorder/TrackSlider.h        (revision 37696)
+++ src/apps/soundrecorder/TrackSlider.h        (working copy)
@@ -14,7 +14,7 @@
 class SliderOffscreenView : public BView {
 
 public:
-       SliderOffscreenView(BRect frame, char *name); 
+       SliderOffscreenView(BRect frame, const char *name); 
        virtual         ~SliderOffscreenView();
        virtual void    DrawX();
        
Index: src/add-ons/media/media-add-ons/firewire_dv/FireWireCard.cpp
===================================================================
--- src/add-ons/media/media-add-ons/firewire_dv/FireWireCard.cpp        
(revision 37696)
+++ src/add-ons/media/media-add-ons/firewire_dv/FireWireCard.cpp        
(working copy)
@@ -75,7 +75,7 @@
        {203, 2997},    /* = (8000 - 29.97 * 250)/(29.97 * 250) */
        {1, 15},        /* = (8000 - 25 * 300)/(25 * 300) */
 };
-char *system_name[] = {(char *)"NTSC", (char *)"PAL"};
+const char *system_name[] = {"NTSC", "PAL"};
 int frame_rate[] = {30, 25};
 
 #define DV_PSIZE 512
Index: src/add-ons/media/media-add-ons/opensound/OpenSoundNode.cpp
===================================================================
--- src/add-ons/media/media-add-ons/opensound/OpenSoundNode.cpp (revision 37696)
+++ src/add-ons/media/media-add-ons/opensound/OpenSoundNode.cpp (working copy)
@@ -485,9 +485,9 @@
                        mediaInput.destination.port = ControlPort();
                        mediaInput.destination.id = fInputs.CountItems();
                        mediaInput.node = Node();
-                       char *prefix = (char *)"";
+                       const char *prefix = "";
                        if (strstr(engine->Info()->name, "SPDIF"))
-                               prefix = (char *)"S/PDIF ";
+                               prefix = "S/PDIF ";
                        sprintf(mediaInput.name, "%sOutput %ld (%s)", prefix,
                                mediaInput.destination.id, 
gSupportedFormatsNames[f]);
 
@@ -533,9 +533,9 @@
                        mediaOutput.source.port = ControlPort();
                        mediaOutput.source.id = fOutputs.CountItems();
                        mediaOutput.node = Node();
-                       char *prefix = (char *)"";
+                       const char *prefix = "";
                        if (strstr(engine->Info()->name, "SPDIF"))
-                               prefix = (char *)"S/PDIF ";
+                               prefix = "S/PDIF ";
                        sprintf(mediaOutput.name, "%sInput %ld (%s)", prefix,
                                mediaOutput.source.id, 
gSupportedFormatsNames[f]);
 
Index: src/add-ons/media/media-add-ons/opensound/OpenSoundAddOn.cpp
===================================================================
--- src/add-ons/media/media-add-ons/opensound/OpenSoundAddOn.cpp        
(revision 37696)
+++ src/add-ons/media/media-add-ons/opensound/OpenSoundAddOn.cpp        
(working copy)
@@ -64,7 +64,7 @@
        fInitCheckStatus = B_NO_INIT;
 
        /* unix paths */
-       if (RecursiveScan((char *)"/dev/oss/") != B_OK)
+       if (RecursiveScan("/dev/oss/") != B_OK)
                return;
        /*
        if (RecursiveScan("/dev/audio/oss/") != B_OK)
@@ -193,7 +193,7 @@
 }
 
 status_t
-OpenSoundAddOn::RecursiveScan(char* rootPath, BEntry *rootEntry)
+OpenSoundAddOn::RecursiveScan(const char* rootPath, BEntry *rootEntry)
 {
        status_t err;
        int mixer;
Index: src/add-ons/media/media-add-ons/opensound/OpenSoundAddOn.h
===================================================================
--- src/add-ons/media/media-add-ons/opensound/OpenSoundAddOn.h  (revision 37696)
+++ src/add-ons/media/media-add-ons/opensound/OpenSoundAddOn.h  (working copy)
@@ -53,7 +53,7 @@
                /************************/
 
        private:
-               status_t RecursiveScan(char* path, BEntry *rootEntry = NULL);
+               status_t RecursiveScan(const char* path, BEntry *rootEntry = 
NULL);
                void SaveSettings(void);
                void LoadSettings(void);
                
Index: src/add-ons/kernel/drivers/graphics/common/log_dump.c
===================================================================
--- src/add-ons/kernel/drivers/graphics/common/log_dump.c       (revision 37696)
+++ src/add-ons/kernel/drivers/graphics/common/log_dump.c       (working copy)
@@ -19,7 +19,7 @@
 system_info sysinfo;
 
 // dump one entry
-void log_printentry( FILE *logfile, log_entry *entry )
+static void log_printentry( FILE *logfile, log_entry *entry )
 {
        uint64 time;
        uint32 min, sec, mill, mic;
Index: src/add-ons/kernel/drivers/graphics/common/log_dump.h
===================================================================
--- src/add-ons/kernel/drivers/graphics/common/log_dump.h       (revision 37696)
+++ src/add-ons/kernel/drivers/graphics/common/log_dump.h       (working copy)
@@ -13,6 +13,5 @@
 #include <SupportDefs.h>
 
 void log_printall( FILE *logfile, char *buffer, uint32 buffer_len );
-void log_printentry( FILE *logfile, log_entry *entry );
 
 #endif
Index: src/add-ons/accelerants/3dfx/accelerant.h
===================================================================
--- src/add-ons/accelerants/3dfx/accelerant.h   (revision 37696)
+++ src/add-ons/accelerants/3dfx/accelerant.h   (working copy)
@@ -185,7 +185,7 @@
 // Write a value to an 32-bit reg using a mask.  The mask selects the
 // bits to be modified.
 #define OUTREGM(addr, value, mask)     \
-       (OUTREG(addr, (INREG(addr) & ~mask) | (value & mask)))
+       (OUTREG(addr, (INREG(addr) & ~(mask)) | ((value) & (mask))))
 
 
 #endif // _ACCELERANT_H
Index: src/add-ons/accelerants/ati/accelerant.h
===================================================================
--- src/add-ons/accelerants/ati/accelerant.h    (revision 37696)
+++ src/add-ons/accelerants/ati/accelerant.h    (working copy)
@@ -202,7 +202,7 @@
 // Write a value to an 32-bit reg using a mask.  The mask selects the
 // bits to be modified.
 #define OUTREGM(addr, value, mask)     \
-       (OUTREG(addr, (INREG(addr) & ~mask) | (value & mask)))
+       (OUTREG(addr, (INREG(addr) & ~(mask)) | ((value) & (mask))))
 
 
 #endif // _ACCELERANT_H
Index: src/add-ons/accelerants/ati/rage128_mode.cpp
===================================================================
--- src/add-ons/accelerants/ati/rage128_mode.cpp        (revision 37696)
+++ src/add-ons/accelerants/ati/rage128_mode.cpp        (working copy)
@@ -304,7 +304,7 @@
 
        OUTREG(R128_CRTC_GEN_CNTL, params.crtc_gen_cntl);
 
-       OUTREGM(R128_DAC_CNTL, (R128_DAC_MASK_ALL | R128_DAC_8BIT_EN),
+       OUTREGM(R128_DAC_CNTL, R128_DAC_MASK_ALL | R128_DAC_8BIT_EN,
                        ~(R128_DAC_RANGE_CNTL | R128_DAC_BLANKING));
 
        OUTREG(R128_CRTC_H_TOTAL_DISP, params.crtc_h_total_disp);
@@ -393,7 +393,7 @@
 
        // Select primary monitor and enable 8-bit color.
        OUTREGM(R128_DAC_CNTL, R128_DAC_8BIT_EN,
-               (R128_DAC_PALETTE_ACC_CTL | R128_DAC_8BIT_EN));
+               R128_DAC_PALETTE_ACC_CTL | R128_DAC_8BIT_EN);
        OUTREG8(R128_PALETTE_INDEX, 0);         // set first color index
 
        for (int i = 0; i < 256; i++)
@@ -436,7 +436,7 @@
 
        // Select primary monitor and enable 8-bit color.
        OUTREGM(R128_DAC_CNTL, R128_DAC_8BIT_EN,
-               (R128_DAC_PALETTE_ACC_CTL | R128_DAC_8BIT_EN));
+               R128_DAC_PALETTE_ACC_CTL | R128_DAC_8BIT_EN);
        OUTREG8(R128_PALETTE_INDEX, first);             // set first color index
 
        while (count--) {
Index: src/add-ons/accelerants/ati/rage128_dpms.cpp
===================================================================
--- src/add-ons/accelerants/ati/rage128_dpms.cpp        (revision 37696)
+++ src/add-ons/accelerants/ati/rage128_dpms.cpp        (working copy)
@@ -72,13 +72,13 @@
                case B_DPMS_STAND_BY:
                        // Screen: Off; HSync: Off, VSync: On.
                        OUTREGM(R128_CRTC_EXT_CNTL,
-                               (R128_CRTC_DISPLAY_DIS | R128_CRTC_HSYNC_DIS), 
mask);
+                               R128_CRTC_DISPLAY_DIS | R128_CRTC_HSYNC_DIS, 
mask);
                        break;
 
                case B_DPMS_SUSPEND:
                        // Screen: Off; HSync: On, VSync: Off.
                        OUTREGM(R128_CRTC_EXT_CNTL,
-                               (R128_CRTC_DISPLAY_DIS | R128_CRTC_VSYNC_DIS), 
mask);
+                               R128_CRTC_DISPLAY_DIS | R128_CRTC_VSYNC_DIS, 
mask);
                        break;
 
                case B_DPMS_OFF:
Index: src/add-ons/accelerants/ati/rage128_init.cpp
===================================================================
--- src/add-ons/accelerants/ati/rage128_init.cpp        (revision 37696)
+++ src/add-ons/accelerants/ati/rage128_init.cpp        (working copy)
@@ -21,10 +21,10 @@
 // Memory Specifications from RAGE 128 Software Development Manual
 // (Technical Reference Manual P/N SDK-G04000 Rev 0.01), page 3-21.
 static R128_RAMSpec sRAMSpecs[] = {
-       { 4, 4, 3, 3, 1, 3, 1, 16, 12, (char *)"128-bit SDR SGRAM 1:1" },
-       { 4, 8, 3, 3, 1, 3, 1, 17, 13, (char *)"64-bit SDR SGRAM 1:1" },
-       { 4, 4, 1, 2, 1, 2, 1, 16, 12, (char *)"64-bit SDR SGRAM 2:1" },
-       { 4, 4, 3, 3, 2, 3, 1, 16, 12, (char *)"64-bit DDR SGRAM" },
+       { 4, 4, 3, 3, 1, 3, 1, 16, 12, "128-bit SDR SGRAM 1:1" },
+       { 4, 8, 3, 3, 1, 3, 1, 17, 13, "64-bit SDR SGRAM 1:1" },
+       { 4, 4, 1, 2, 1, 2, 1, 16, 12, "64-bit SDR SGRAM 2:1" },
+       { 4, 4, 3, 3, 2, 3, 1, 16, 12, "64-bit DDR SGRAM" },
 };
 
 
Index: src/add-ons/mail_daemon/inbound_filters/spam_filter/SpamFilterConfig.cpp
===================================================================
--- src/add-ons/mail_daemon/inbound_filters/spam_filter/SpamFilterConfig.cpp    
(revision 37696)
+++ src/add-ons/mail_daemon/inbound_filters/spam_filter/SpamFilterConfig.cpp    
(working copy)
@@ -167,7 +167,7 @@
 {
        char             numberString [30];
        BRect            tempRect;
-       char            *tempStringPntr;
+       const char              *tempStringPntr;
 
        SetViewColor (ui_color (B_PANEL_BACKGROUND_COLOR));
 
@@ -209,7 +209,7 @@
 
        tempRect.right = fNoWordsMeansSpamCheckBoxPntr->Frame().left -
                be_plain_font->StringWidth ("a");
-       tempStringPntr = (char *)"Spam above:";
+       tempStringPntr = "Spam above:";
        sprintf (numberString, "%06.4f", (double) fSpamCutoffRatio);
        fSpamCutoffRatioTextBoxPntr     = new BTextControl (
                tempRect,
@@ -228,7 +228,7 @@
 
        // Add the box displaying the genuine cutoff ratio, on a line by itself.
 
-       tempStringPntr = (char *)"Genuine below and uncertain above:";
+       tempStringPntr = "Genuine below and uncertain above:";
        sprintf (numberString, "%08.6f", (double) fGenuineCutoffRatio);
        fGenuineCutoffRatioTextBoxPntr = new BTextControl (
                tempRect,
Index: headers/private/graphics/ati/DriverInterface.h
===================================================================
--- headers/private/graphics/ati/DriverInterface.h      (revision 37696)
+++ headers/private/graphics/ati/DriverInterface.h      (working copy)
@@ -145,7 +145,7 @@
        int  readToWriteDelay;  // Read to Write Delay
        int  loopLatency;               // Loop Latency
        int  loopFudgeFactor;   // Add to memReadLatency to get loopLatency
-       char *name;
+       const char *name;
 };
 
 

Other related posts: