[opensuse-programming] Need help 'fixing' some code
Hi everybody,
I'm not really a coder, I'm packager... but with the latest checks in OBS, fixing code get's a 'normal' task as well.
Today I stumble upon a package that fails in this code segment:
// 5: 16-bit signed PCM data
case RS_PCM16S:
{
len = pIns->nLength * 2;
if (len <= dwMemLength) memcpy(pIns->pSample, lpMemFile, len);
short int *pSample = (short int *)pIns->pSample;
for (UINT j=0; j
* Dominique Leuenberger [2008-12-19 11:54]:
{ *pSample++ = bswapLE16(*pSample); }
[...]
Would splitting it into *pSample = bswapLE16(*pSample); *pSample++;
I think. Actually you need only *pSample = bswapLE16(*pSample); pSample++; Bernhard -- Bernhard Walle, SUSE Linux Products GmbH, Architecture Development -- To unsubscribe, e-mail: opensuse-programming+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-programming+help@opensuse.org
>>> On 12/19/2008 at 12:01 PM, Bernhard Wallewrote: > * Dominique Leuenberger [2008-12-19 11:54]: >> { >> *pSample++ = bswapLE16(*pSample); >> } > [...] >> Would splitting it into >> *pSample = bswapLE16(*pSample); >> *pSample++; > > I think. Actually you need only > > *pSample = bswapLE16(*pSample); > pSample++; Oh, right ;) I'll give it a try and hope it really is, what the compiler does out of this instructions in normal cases. The line is indeed not very 'clear' to understand and leaves room for interpretation. Dominique -- Dominique Leuenberger TMF Netherlands B.V Locatellikade 1, 1070 AZ Amsterdam E-mail: dominique.leuenberger@tmf-group.com www.tmf-group.com -- To unsubscribe, e-mail: opensuse-programming+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-programming+help@opensuse.org
* Dominique Leuenberger (Dominique.Leuenberger@TMF-Group.com) [20081219 11:59]:
*pSample++ = bswapLE16(*pSample); Looks already suspicious...
Would splitting it into *pSample = bswapLE16(*pSample); *pSample++; Actually have the effect as intended originally? Or is it impossible to really tell this?
This is exactly what the original intended. Earlier gccs didn't flag this but the the order is undefined so a compiler is also allowed to do *pSample++; bswapLE16(*pS2ample); which would be wrong. Philipp -- To unsubscribe, e-mail: opensuse-programming+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-programming+help@opensuse.org
>>> On 12/19/2008 at 2:22 PM, Philipp Thomaswrote: > * Dominique Leuenberger (Dominique.Leuenberger@TMF-Group.com) [20081219 > 11:59]: > >> *pSample++ = bswapLE16(*pSample); >> Looks already suspicious... >> > >> Would splitting it into >> *pSample = bswapLE16(*pSample); >> *pSample++; >> Actually have the effect as intended originally? Or is it impossible to > really tell this? > > This is exactly what the original intended. Earlier gccs didn't flag this > but the the order is undefined so a compiler is also allowed to do > > *pSample++; > bswapLE16(*pS2ample); > > which would be wrong. > > Philipp Philip, Thank you very much for this explanation. I created the patch according this information. I was especially troubled with the incrementor being in front of the equal sign... but by pure logic and understanding of it, it has to be executed after the assignment.. so I assume it's correct ;). Thank you very much... I have some other ones ;) (but will be a new thread). Dominique -- To unsubscribe, e-mail: opensuse-programming+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-programming+help@opensuse.org
participants (3)
-
Bernhard Walle
-
Dominique Leuenberger
-
Philipp Thomas