[Qemu-devel] [PATCH] cocoa.m issues fixed

[Qemu-devel] [PATCH] cocoa.m issues fixed

[G 3]

This patch allows the file cocoa.m to compile under Mac OS 10.3.  
MAC_OS_X_VERSION_10_4 isn't defined under Mac OS 10.3. The #define will 
define it. The method cStringUsingEncoding isn't defined under Mac OS 
10.3, so I used a similar function that does work.

Signed-off-by: programmingkid  <programmingkidx@gmail.com>
--- cocoa.m	Wed May 20 16:46:58 2009
+++ cocoa (edited).m	Sat Jun 20 20:57:41 2009
@@ -28,6 +28,13 @@
  #include "console.h"
  #include "sysemu.h"

+#ifndef MAC_OS_X_VERSION_10_4
+#define MAC_OS_X_VERSION_10_4 1040
+#endif
+
+#ifndef __LITTLE_ENDIAN__
+#define __LITTLE_ENDIAN__ 0  /* assume PowerPC*/
+#endif

  //#define DEBUG

@@ -55,6 +62,8 @@
  } QEMUScreen;

  int qemu_main(int argc, char **argv); // main defined in qemu/vl.c
+int cocoa_keycode_to_qemu(int keycode);
+
  NSWindow *normalWindow;
  id cocoaView;
  static DisplayChangeListener *dcl;
@@ -783,8 +792,8 @@
      if(returnCode == NSCancelButton) {
          exit(0);
      } else if(returnCode == NSOKButton) {
-        char *bin = "qemu";
-        char *img = (char*)[ [ sheet filename ] 
cStringUsingEncoding:NSASCIIStringEncoding];
+        char bin[5] = "qemu";
+        char *img = (char*)[ [ sheet filename ] cString];

          char **argv = (char**)malloc( sizeof(char*)*3 );

Re: [Qemu-devel] [PATCH] cocoa.m issues fixed

[Andreas Färber]

Am 21.06.2009 um 03:19 schrieb G 3:

> This patch allows the file cocoa.m to compile under Mac OS 10.3.   
> MAC_OS_X_VERSION_10_4 isn't defined under Mac OS 10.3. The #define  
> will define it. The method cStringUsingEncoding isn't defined under  
> Mac OS 10.3, so I used a similar function that does work.
>
> Signed-off-by: programmingkid  <programmingkidx@gmail.com>
> --- cocoa.m	Wed May 20 16:46:58 2009
> +++ cocoa (edited).m	Sat Jun 20 20:57:41 2009

> @@ -55,6 +62,8 @@
> } QEMUScreen;
>
> int qemu_main(int argc, char **argv); // main defined in qemu/vl.c
> +int cocoa_keycode_to_qemu(int keycode);
> +

This seems unrelated. I believe you're trying to suppress a warning  
I've been seeing on 10.5 as well - if so, please provide that as a  
separate patch with appropriate description.

> NSWindow *normalWindow;
> id cocoaView;
> static DisplayChangeListener *dcl;
> @@ -783,8 +792,8 @@
>     if(returnCode == NSCancelButton) {
>         exit(0);
>     } else if(returnCode == NSOKButton) {
> -        char *bin = "qemu";
> -        char *img = (char*)[ [ sheet filename ]  
> cStringUsingEncoding:NSASCIIStringEncoding];
> +        char bin[5] = "qemu";

What warning/error does this solve?

> +        char *img = (char*)[ [ sheet filename ] cString];
>
>         char **argv = (char**)malloc( sizeof(char*)*3 );

Andreas

Re: [Qemu-devel] [PATCH] cocoa.m issues fixed

[G 3]

On Jun 21, 2009, at 6:10 AM, Andreas Färber wrote:

>
> Am 21.06.2009 um 03:19 schrieb G 3:
>
>> +int cocoa_keycode_to_qemu(int keycode);
>> +
>
> This seems unrelated. I believe you're trying to suppress a warning 
> I've been seeing on 10.5 as well - if so, please provide that as a 
> separate patch with appropriate description.
>

This function prototype would eliminate this warning:
cocoa.m:233: warning: no previous prototype for `cocoa_keycode_to_qemu'

Why a separate patch. Why not kill two birds with one stone?



>> +        char bin[5] = "qemu";
>
> What warning/error does this solve?

This is the warning that this change resolves:
cocoa.m:786: warning: initialization discards qualifiers from pointer 
target type

Re: [Qemu-devel] [PATCH] cocoa.m issues fixed

[Avi Kivity]

On 06/21/2009 05:06 PM, G 3 wrote:
>
> On Jun 21, 2009, at 6:10 AM, Andreas Färber wrote:
>
>>
>> Am 21.06.2009 um 03:19 schrieb G 3:
>>
>>> +int cocoa_keycode_to_qemu(int keycode);
>>> +
>>
>> This seems unrelated. I believe you're trying to suppress a warning 
>> I've been seeing on 10.5 as well - if so, please provide that as a 
>> separate patch with appropriate description.
>>
>
> This function prototype would eliminate this warning:
> cocoa.m:233: warning: no previous prototype for `cocoa_keycode_to_qemu'
>
> Why a separate patch. Why not kill two birds with one stone?

It's standard operating procedure.  Suppose in addition to the two birds 
you mention the patch also kills an innocent kitten.  Since it's one 
patch, if a fix is not immediately forthcoming, the maintainer has to 
revert the patch, bringing both birds back to life.

With one patch per bird, the maintainer can revert just the patch which 
killed the kitten, leaving the other bird dead.

It's also easier to review two small patches rather than one large patch.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] cocoa.m issues fixed

[Blue Swirl]

On 6/21/09, Avi Kivity <avi@redhat.com> wrote:
> On 06/21/2009 05:06 PM, G 3 wrote:
>
> >
> > On Jun 21, 2009, at 6:10 AM, Andreas Färber wrote:
> >
> >
> > >
> > > Am 21.06.2009 um 03:19 schrieb G 3:
> > >
> > >
> > > > +int cocoa_keycode_to_qemu(int keycode);
> > > > +
> > > >
> > >
> > > This seems unrelated. I believe you're trying to suppress a warning I've
> been seeing on 10.5 as well - if so, please provide that as a separate patch
> with appropriate description.
> > >
> > >
> >
> > This function prototype would eliminate this warning:
> > cocoa.m:233: warning: no previous prototype for `cocoa_keycode_to_qemu'
> >
> > Why a separate patch. Why not kill two birds with one stone?
> >
>
>  It's standard operating procedure.  Suppose in addition to the two birds
> you mention the patch also kills an innocent kitten.  Since it's one patch,
> if a fix is not immediately forthcoming, the maintainer has to revert the
> patch, bringing both birds back to life.

How nasty for you to forget the poor little kitten, who should be
entitled to get her life back.

>  With one patch per bird, the maintainer can revert just the patch which
> killed the kitten, leaving the other bird dead.

Also here, do you really hate kittens that much? ;-)

Re: [Qemu-devel] [PATCH] cocoa.m issues fixed

[Avi Kivity]

On 06/21/2009 09:07 PM, Blue Swirl wrote:
> This function prototype would eliminate this warning:
>>> cocoa.m:233: warning: no previous prototype for `cocoa_keycode_to_qemu'
>>>
>>> Why a separate patch. Why not kill two birds with one stone?
>>>
>>>        
>>   It's standard operating procedure.  Suppose in addition to the two birds
>> you mention the patch also kills an innocent kitten.  Since it's one patch,
>> if a fix is not immediately forthcoming, the maintainer has to revert the
>> patch, bringing both birds back to life.
>>      
>
> How nasty for you to forget the poor little kitten, who should be
> entitled to get her life back.
>    

Um, by reverting the patch, the kitten is as good as new.

>>   With one patch per bird, the maintainer can revert just the patch which
>> killed the kitten, leaving the other bird dead.
>>      
>
> Also here, do you really hate kittens that much? ;-)
>    

Here too.  The kitten is alive and well, as long as we can keep 
Schroedinger's mitts off of her.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH] cocoa.m issues fixed

[Stuart Brady]

On Sun, Jun 21, 2009 at 09:07:06PM +0300, Blue Swirl wrote:
> On 6/21/09, Avi Kivity <avi@redhat.com> wrote:
> >  It's standard operating procedure.  Suppose in addition to the two birds
> > you mention the patch also kills an innocent kitten.  Since it's one patch,
> > if a fix is not immediately forthcoming, the maintainer has to revert the
> > patch, bringing both birds back to life.
> 
> How nasty for you to forget the poor little kitten, who should be
> entitled to get her life back.

Why are the birds being brought back to life, anyway?  I thouht they idea
was to kill two birds with one stone?

Also, what have people got against bird all of a sudden?

Consider the lily?!  He's having a go at the flowers, now! ;-)

> >  With one patch per bird, the maintainer can revert just the patch which
> > killed the kitten, leaving the other bird dead.
> 
> Also here, do you really hate kittens that much? ;-)

Or birds, for that matter...

BTW, other reasons to apply one fix per patch, in no particular order:

 * Makes resolving merge conflicts more straightforward in certain
   cases, as it's easier to see why different lines were modified.
   (I.e. multiple changes with a long description weakens the tie
   between the description and the individual fixes that were applied.)

 * Means that when looking at commits, people can see the changes that 
   they're interested in, instead of having to manually skip past all
   the cosmetic stuff that they're not interested in.

 * Means that the fix can be cherry picked for a distribution easily,
   without forcing maintainers to filter out cosmetic changes that 
   shouldn't be made to stable release.

 * Increases the chance of cleanups being made in all cases, rather than
   just the one-or-two that you happened to spot in whatever file or
   function you were dealing with at the time.

 * Means that if there's a problem with only one change out of many,
   it's likely that you'll only need to resubmit the change that was
   incorrect.  (This also eliminates/reduces the chance of any sneaky
   or even unintended changes being made regarding the other fixes.)

 * Makes changes stand out better in the shortlog.

 * Improves the likelihood that maintainers will consider your patch
   to be reviewable, and then actually review it and apply it, if those
   maintainers lack the time to review all patches that are submitted.

I'm sure there are many more reasons.

For one simple patch, it may not seem like a big deal, but for several
hundred simple patches, it is.

Cheers,
-- 
Stuart Brady

Re: [Qemu-devel] [PATCH] cocoa.m issues fixed

[Blue Swirl]

On 6/22/09, Stuart Brady <sdbrady@ntlworld.com> wrote:
> On Sun, Jun 21, 2009 at 09:07:06PM +0300, Blue Swirl wrote:
>  > On 6/21/09, Avi Kivity <avi@redhat.com> wrote:
>
> > >  It's standard operating procedure.  Suppose in addition to the two birds
>  > > you mention the patch also kills an innocent kitten.  Since it's one patch,
>  > > if a fix is not immediately forthcoming, the maintainer has to revert the
>  > > patch, bringing both birds back to life.
>  >
>  > How nasty for you to forget the poor little kitten, who should be
>  > entitled to get her life back.
>
>
> Why are the birds being brought back to life, anyway?  I thouht they idea
>  was to kill two birds with one stone?

Yes, but since the maintainer reversed the patch, the birds and the
kitten were magically resurrected.

>  Also, what have people got against bird all of a sudden?

I'm afraid you must consult with your ancestors, who invited this bird
killing talk and other monstrosities.

>  Consider the lily?!  He's having a go at the flowers, now! ;-)

ENOPARSE

>  > >  With one patch per bird, the maintainer can revert just the patch which
>  > > killed the kitten, leaving the other bird dead.
>  >
>  > Also here, do you really hate kittens that much? ;-)
>
>
> Or birds, for that matter...
>
>  BTW, other reasons to apply one fix per patch, in no particular order:
>
>   * Makes resolving merge conflicts more straightforward in certain
>    cases, as it's easier to see why different lines were modified.
>    (I.e. multiple changes with a long description weakens the tie
>    between the description and the individual fixes that were applied.)
>
>   * Means that when looking at commits, people can see the changes that
>    they're interested in, instead of having to manually skip past all
>    the cosmetic stuff that they're not interested in.
>
>   * Means that the fix can be cherry picked for a distribution easily,
>    without forcing maintainers to filter out cosmetic changes that
>    shouldn't be made to stable release.
>
>   * Increases the chance of cleanups being made in all cases, rather than
>    just the one-or-two that you happened to spot in whatever file or
>    function you were dealing with at the time.
>
>   * Means that if there's a problem with only one change out of many,
>    it's likely that you'll only need to resubmit the change that was
>    incorrect.  (This also eliminates/reduces the chance of any sneaky
>    or even unintended changes being made regarding the other fixes.)
>
>   * Makes changes stand out better in the shortlog.
>
>   * Improves the likelihood that maintainers will consider your patch
>    to be reviewable, and then actually review it and apply it, if those
>    maintainers lack the time to review all patches that are submitted.

+1 for this one.

>  I'm sure there are many more reasons.
>
>  For one simple patch, it may not seem like a big deal, but for several
>  hundred simple patches, it is.

Yes, imagine a Beowulf cluster of... Nevermind.

Re: [Qemu-devel] [PATCH] cocoa.m issues fixed

[Stuart Brady]

On Mon, Jun 22, 2009 at 08:35:29PM +0300, Blue Swirl wrote:
> On 6/22/09, Stuart Brady <sdbrady@ntlworld.com> wrote:
> > Why are the birds being brought back to life, anyway?  I thouht they idea
> >  was to kill two birds with one stone?
> 
> Yes, but since the maintainer reversed the patch, the birds and the
> kitten were magically resurrected.

So you mean, bring the kitten and the birds back to life, and then kill
the birds again?  *Mind boggles*.  Ah.  Perhaps that was the point. :-)

(I see now that the birds would have to be revived, by necessity rather
than by choice, unless the correct stones are found in a timely manner.)

> >  Consider the lily?!  He's having a go at the flowers, now! ;-)
> 
> ENOPARSE

Just a reference to Monty Python's Life of Brian, is all. :-)

> >   * Improves the likelihood that maintainers will consider your patch
> >    to be reviewable, and then actually review it and apply it, if those
> >    maintainers lack the time to review all patches that are submitted.
> 
> +1 for this one.

Ah, I thought that one might prove popular. :-)

Cheers,
-- 
Stuart Brady