[opensuse-programming] Need help 'fixing' some code
![](https://seccdn.libravatar.org/avatar/c2245049e7e6a67166114fef782634e3.jpg?s=120&d=mm&r=g)
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<len; j+=2) { *pSample++ = bswapLE16(*pSample); } } break; The error BS gives is: I: Program causes undefined operation (likely same variable used twiceand post/pre incremented in the same expression). e.g. x = x++; Split it in two operations. E: packageX sequence-point file.cpp:1184, 1375 Of course the line *pSample++ = bswapLE16(*pSample); Looks already suspicious... I would not even know what the real author was trying to achieve... and I'd consider it a wonder this stayed in for so long. What would, according to you, be the proper solution / patch for this? Would splitting it into *pSample = bswapLE16(*pSample); *pSample++; Actually have the effect as intended originally? Or is it impossible to really tell this? Dominique -- To unsubscribe, e-mail: opensuse-programming+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-programming+help@opensuse.org
![](https://seccdn.libravatar.org/avatar/95bf8795d2703f0024d688dc0dfa2ad5.jpg?s=120&d=mm&r=g)
* 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
![](https://seccdn.libravatar.org/avatar/c2245049e7e6a67166114fef782634e3.jpg?s=120&d=mm&r=g)
>>> On 12/19/2008 at 12:01 PM, Bernhard Walle <bwalle@suse.de> wrote: > * 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
![](https://seccdn.libravatar.org/avatar/b1966ffac861531a1d6494248e650a12.jpg?s=120&d=mm&r=g)
* 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
![](https://seccdn.libravatar.org/avatar/c2245049e7e6a67166114fef782634e3.jpg?s=120&d=mm&r=g)
>>> On 12/19/2008 at 2:22 PM, Philipp Thomas <pth@suse.de> wrote: > * 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