Re: [Qemu-devel] [PATCH 1/1 v4] Allow machines to configure the QEMU_VERSION that's exposed via hardware

2012-05-24 Thread Crístian Viana
On 23-05-2012 17:54, Peter Maydell wrote: The point is that your snprintf is not actually using the full power of a format string parser, it's just concatenating two strings ("QEMU " and the version). The simple way to put two strings into a buffer one after the other is to copy string A and then

Re: [Qemu-devel] [PATCH 1/1 v4] Allow machines to configure the QEMU_VERSION that's exposed via hardware

2012-05-23 Thread Peter Maydell
On 23 May 2012 21:06, Crístian Viana wrote: > This would be the new code: > > snprintf((void *) w, 12, "QEMU %s", qemu_get_version()); /* char version[12] > */ > > I'm not sure of what value the pointer contains at that moment, > concatenating doesn't seem safe to me. What if w already contains a

Re: [Qemu-devel] [PATCH 1/1 v4] Allow machines to configure the QEMU_VERSION that's exposed via hardware

2012-05-23 Thread Crístian Viana
On 23-05-2012 13:11, Eric Blake wrote: pstrcat is more efficient than snprintf() - the former is dedicated to a single task, while the latter has to parse a format string and decode that it is doing a single %s expansion. In other words, just because *printf can do string concatenation doesn't m

Re: [Qemu-devel] [PATCH 1/1 v4] Allow machines to configure the QEMU_VERSION that's exposed via hardware

2012-05-23 Thread Eric Blake
On 05/23/2012 10:08 AM, Crístian Viana wrote: >> So when you posted the previous version of your patch it was pointed >> out that this is a buffer overflow: >> http://lists.gnu.org/archive/html/qemu-devel/2012-04/msg01657.html >> >> You need to fix this. > > I have sent a reply to that thread exp

Re: [Qemu-devel] [PATCH 1/1 v4] Allow machines to configure the QEMU_VERSION that's exposed via hardware

2012-05-23 Thread Crístian Viana
Hi Peter, Thanks for all your tips! OK, this has been bugging me for the last three versions, and since I'm complaining about other things anyway: can you reword this commit message, please, so that it is a standalone paragraph explaining (a) what the commit does and (b) why it is doing it, rat

Re: [Qemu-devel] [PATCH 1/1 v4] Allow machines to configure the QEMU_VERSION that's exposed via hardware

2012-05-22 Thread Peter Maydell
On 22 May 2012 22:10, Crístian Viana wrote: > Based on the following conversation: > > http://mid.gmane.org/4f69f05b.5010...@codemonkey.ws > >> Which reminds me - qemu sticks the release version in >> guest visible places like CPU version. >> This is wrong and causes windows guests to print messag

[Qemu-devel] [PATCH 1/1 v4] Allow machines to configure the QEMU_VERSION that's exposed via hardware

2012-05-22 Thread Crístian Viana
Based on the following conversation: http://mid.gmane.org/4f69f05b.5010...@codemonkey.ws > Which reminds me - qemu sticks the release version in > guest visible places like CPU version. > This is wrong and causes windows guests to print messages > about driver updates when you switch. > We should