Re: [janitor] dlls/winmm/wine* -Wwrite-strings cleanup

[Jerry Jenkins]

> Index: dlls/winmm/winealsa/audio.c
>
> RCS file: /home/wine/wine/dlls/winmm/winealsa/audio.c,v
> retrieving revision 1.20
> diff -u -r1.20 audio.c
> --- dlls/winmm/winealsa/audio.c	22 Sep 2003 21:13:33 -0000	1.20
> +++ dlls/winmm/winealsa/audio.c	7 Oct 2003 19:52:14 -0000
> @@ -408,11 +408,12 @@
>      snd_pcm_info_t *            info;
>      snd_pcm_hw_params_t *       hw_params;
>      WINE_WAVEOUT*	        wwo;
> - char			device[] = "hw";

I think it should be
static char			device[] = "hw";
>  wwo = &WOutDev[0];
>
>  /* FIXME: use better values */
> - wwo->device = "hw";
> - wwo->device = device;
> wwo->caps.wMid = 0x0002;
> wwo->caps.wPid = 0x0104;
> strcpy(wwo->caps.szPname, "SB16 Wave Out");

[janitor] dlls/winmm/wine* -Wwrite-strings cleanup

[Dimitrie O. Paun]

On October 11, 2003 03:42 am, Jerry Jenkins wrote:
> > - char                    device[] = "hw";
>
> I think it should be
> static char                     device[] = "hw";

Why is that? In fact, I think it shouldn't, if it
gets modified during the call, we'll end up passing
garbage values on subsequent calls...

[janitor] dlls/winmm/wine* -Wwrite-strings cleanup

[Sylvain Petreolle]

Was something like this already applied to CVS ?
This could explain why I got garbage into winealsa for the device
names.
> > > - char                    device[] = "hw";
> >
> > I think it should be
> > static char                     device[] = "hw";
>
> Why is that? In fact, I think it shouldn't, if it
> gets modified during the call, we'll end up passing
> garbage values on subsequent calls...

=====
Sylvain Petreolle (spetreolle_at_users_dot_sourceforge_dot_net)
 ICQ #170597259
Say NO to software patents
Dites NON aux brevets logiciels

"What if tomorrow the War could be over ?" Morpheus, in "Reloaded".

___________________________________________________________
Do You Yahoo!? -- Une adresse @yahoo.fr gratuite et en français !
Yahoo! Mail : http://fr.mail.yahoo.com

[janitor] dlls/winmm/wine* -Wwrite-strings cleanup

[Daniel Marmier]

On Sun, 2003-10-12 at 05:19, Sylvain Petreolle wrote:
> Was something like this already applied to CVS ?
> This could explain why I got garbage into winealsa for the device
> names.
> > > > - char                    device[] = "hw";
> > >
> > > I think it should be
> > > static char                     device[] = "hw";

Yes, I submitted one such change in winealsa/audio.c
(wine-20031007-dm8.diff). But it was a non-static one, so it does not
explain the garbage you get.

Best Regards,

Daniel Marmier

[janitor] dlls/winmm/wine* -Wwrite-strings cleanup

[Jerry Jenkins]

Dimitrie O. Paun wrote:
> On October 11, 2003 03:42 am, Jerry Jenkins wrote:
> > > - char                    device[] = "hw";
> >
> > I think it should be
> > static char                     device[] = "hw";
>
> Why is that? In fact, I think it shouldn't, if it
> gets modified during the call, we'll end up passing
> garbage values on subsequent calls...

Actually device[] will and shall not be modified at least up to now. It 
is introduced by the patch, though I though that we can get rid of the 
warning by changing the type of device member in structure WINE_WAVEOUT 
from char * to const char *. If we keep device[], we need the static 
keyword to make winealsa work.

[janitor] dlls/winmm/wine* -Wwrite-strings cleanup

[Dimitrie O. Paun]

On October 12, 2003 01:00 am, Jerry Jenkins wrote:
> If we keep device[], we need the static
> keyword to make winealsa work.

Why do we need the static?

[janitor] dlls/winmm/wine* -Wwrite-strings cleanup

[Jerry Jenkins]

Dimitrie O. Paun wrote:
> Why do we need the static?

Had I pasted more source code, you would have known why.
LONG ALSA_WaveInit(void)
{
     snd_pcm_t*                  h = NULL;
     snd_pcm_info_t *            info;
     snd_pcm_hw_params_t *       hw_params;
     WINE_WAVEOUT*	        wwo;
     char			device[] = "hw";

wwo = &WOutDev[0];

/* FIXME: use better values */
     wwo->device = device;
We have to keep the memory used by device[] or wwo->device even when we 
exit the function, because we'll access it later. Local variables’ 
memory are supposed to be freed after the function exits.

[janitor] dlls/winmm/wine* -Wwrite-strings cleanup

[Daniel Marmier]

On Sun, 2003-10-12 at 13:04, Jerry Jenkins wrote:
> Dimitrie O. Paun wrote:
> > Why do we need the static?
>
> Had I pasted more source code, you would have known why.
> LONG ALSA_WaveInit(void)
> {
>      snd_pcm_t*                  h = NULL;
>      snd_pcm_info_t *            info;
>      snd_pcm_hw_params_t *       hw_params;
>      WINE_WAVEOUT*	        wwo;
>      char			device[] = "hw";
>  wwo = &WOutDev[0];
>
>  /* FIXME: use better values */
>  wwo->device = device;
>
> We have to keep the memory used by device[] or wwo->device even when we 
> exit the function, because we'll access it later. Local variables’ 
> memory are supposed to be freed after the function exits.

Sorry for breaking it. I noticed the string never gets modified, so I
fixed it by casting in a recent patch.

Best Regards,

Daniel Marmier