Patchwork win32: Add missing function setenv

login
register
mail settings
Submitter Stefan Weil
Date July 1, 2010, 10:47 a.m.
Message ID <1277981269-751-1-git-send-email-weil@mail.berlios.de>
Download mbox | patch
Permalink /patch/57532/
State New
Headers show

Comments

Stefan Weil - July 1, 2010, 10:47 a.m.
Mingw32 does not provide a declaration and implementation of function
setenv (which is used in sdl.c), so this patch adds both.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 os-win32.c |   15 +++++++++++++++
 osdep.h    |    2 ++
 2 files changed, 17 insertions(+), 0 deletions(-)
Jes Sorensen - July 1, 2010, 11:50 a.m.
On 07/01/10 12:47, Stefan Weil wrote:
> Mingw32 does not provide a declaration and implementation of function
> setenv (which is used in sdl.c), so this patch adds both.
> 
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
[snip]
> diff --git a/osdep.h b/osdep.h
> index 75b5816..1cdc7e2 100644
> --- a/osdep.h
> +++ b/osdep.h
> @@ -95,6 +95,8 @@ int qemu_create_pidfile(const char *filename);
>  #ifdef _WIN32
>  int ffs(int i);
>  
> +int setenv(const char *name, const char *value, int overwrite);
> +
>  typedef struct {
>      long tv_sec;
>      long tv_usec;

Please move this to qemu-os-win32.h instead, otherwise the build will
fail on POSIX systems due to setenv being redefined.

Thanks,
Jes
Stefan Weil - July 1, 2010, 1:22 p.m.
Am 01.07.2010 13:50, schrieb Jes Sorensen:
> On 07/01/10 12:47, Stefan Weil wrote:
>    
>> Mingw32 does not provide a declaration and implementation of function
>> setenv (which is used in sdl.c), so this patch adds both.
>>
>> Signed-off-by: Stefan Weil<weil@mail.berlios.de>
>>      
> [snip]
>    
>> diff --git a/osdep.h b/osdep.h
>> index 75b5816..1cdc7e2 100644
>> --- a/osdep.h
>> +++ b/osdep.h
>> @@ -95,6 +95,8 @@ int qemu_create_pidfile(const char *filename);
>>   #ifdef _WIN32
>>   int ffs(int i);
>>
>> +int setenv(const char *name, const char *value, int overwrite);
>> +
>>   typedef struct {
>>       long tv_sec;
>>       long tv_usec;
>>      
> Please move this to qemu-os-win32.h instead, otherwise the build will
> fail on POSIX systems due to setenv being redefined.
>
> Thanks,
> Jes
>    

It won't fail for two reasons:

* It is not redefined (at least for linux systems) because I used the 
POSIX declaration.

* It is compiled only for _WIN32 (see line 95).

Regards
Stefan
Jes Sorensen - July 1, 2010, 1:24 p.m.
On 07/01/10 15:22, Stefan Weil wrote:
> It won't fail for two reasons:
> 
> * It is not redefined (at least for linux systems) because I used the
> POSIX declaration.

This still fails with strict compiler flags.

> * It is compiled only for _WIN32 (see line 95).

True, but we need to move stuff out of osdep.h and into the other files
as much as possible, so it is still preferred that you move it.

Cheers,
Jes
Stefan Weil - July 1, 2010, 3:51 p.m.
Am 01.07.2010 15:24, schrieb Jes Sorensen:
> On 07/01/10 15:22, Stefan Weil wrote:
>> It won't fail for two reasons:
>>
>> * It is not redefined (at least for linux systems) because I used the
>> POSIX declaration.
>
> This still fails with strict compiler flags.
>
>> * It is compiled only for _WIN32 (see line 95).
>
> True, but we need to move stuff out of osdep.h and into the other files
> as much as possible, so it is still preferred that you move it.

That's a valid argument. As there is more stuff to move out of osdep.h,
I suggest doing that in a second step. Now, it is most important to get
Windows builds working again (they fail currently with a linker error).

>
> Cheers,
> Jes
>

Cheers,
Stefan
Jes Sorensen - July 1, 2010, 3:53 p.m.
On 07/01/10 17:51, Stefan Weil wrote:
> Am 01.07.2010 15:24, schrieb Jes Sorensen:
>> On 07/01/10 15:22, Stefan Weil wrote:
>>> It won't fail for two reasons:
>>>
>>> * It is not redefined (at least for linux systems) because I used the
>>> POSIX declaration.
>>
>> This still fails with strict compiler flags.
>>
>>> * It is compiled only for _WIN32 (see line 95).
>>
>> True, but we need to move stuff out of osdep.h and into the other files
>> as much as possible, so it is still preferred that you move it.
> 
> That's a valid argument. As there is more stuff to move out of osdep.h,
> I suggest doing that in a second step. Now, it is most important to get
> Windows builds working again (they fail currently with a linker error).

Rather than add it to remove it in the next patch, please update your
patch and repost it. That causes less noise and will get win32 building
again just as fast.

Cheers,
Jes
Stefan Weil - July 1, 2010, 5:53 p.m.
Am 01.07.2010 17:53, schrieb Jes Sorensen:
> On 07/01/10 17:51, Stefan Weil wrote:
>    
>> Am 01.07.2010 15:24, schrieb Jes Sorensen:
>>      
>>> On 07/01/10 15:22, Stefan Weil wrote:
>>>        
>>>> It won't fail for two reasons:
>>>>
>>>> * It is not redefined (at least for linux systems) because I used the
>>>> POSIX declaration.
>>>>          
>>> This still fails with strict compiler flags.
>>>
>>>        
>>>> * It is compiled only for _WIN32 (see line 95).
>>>>          
>>> True, but we need to move stuff out of osdep.h and into the other files
>>> as much as possible, so it is still preferred that you move it.
>>>        
>> That's a valid argument. As there is more stuff to move out of osdep.h,
>> I suggest doing that in a second step. Now, it is most important to get
>> Windows builds working again (they fail currently with a linker error).
>>      
> Rather than add it to remove it in the next patch, please update your
> patch and repost it. That causes less noise and will get win32 building
> again just as fast.
>
> Cheers,
> Jes
>    

Two patches are needed anyway.

For reasons of economy, I won't send a new patch.
Feel free do send one which meets your criteria.

Regards,
Stefan
Jes Sorensen - July 2, 2010, 8:40 a.m.
On 07/01/10 19:53, Stefan Weil wrote:
> Two patches are needed anyway.
> 
> For reasons of economy, I won't send a new patch.
> Feel free do send one which meets your criteria.
> 
> Regards,
> Stefan
> 

Well if you are not interested in working the way the community works,
please don't expect us to fix your bugs either.

Jes

Patch

diff --git a/os-win32.c b/os-win32.c
index d98fd77..dd46bf4 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -34,6 +34,21 @@ 
 #include "qemu-options.h"
 
 /***********************************************************/
+/* Functions missing in mingw */
+
+int setenv(const char *name, const char *value, int overwrite)
+{
+    int result = 0;
+    if (overwrite || !getenv(name)) {
+        size_t length = strlen(name) + strlen(value) + 2;
+        char *string = qemu_malloc(length);
+        snprintf(string, length, "%s=%s", name, value);
+        result = putenv(string);
+    }
+    return result;
+}
+
+/***********************************************************/
 /* Polling handling */
 
 typedef struct PollingEntry {
diff --git a/osdep.h b/osdep.h
index 75b5816..1cdc7e2 100644
--- a/osdep.h
+++ b/osdep.h
@@ -95,6 +95,8 @@  int qemu_create_pidfile(const char *filename);
 #ifdef _WIN32
 int ffs(int i);
 
+int setenv(const char *name, const char *value, int overwrite);
+
 typedef struct {
     long tv_sec;
     long tv_usec;