larmbr zhan wrote:
> On Sun, Mar 10, 2013 at 5:48 AM, Andrew Talbot
> wrote:
>> msvcp60: Avoid signed-unsigned integer comparisons.
>
>
> Hi, Andrew Talbot.
>
> I find that you are working on these "Avoid signed-unsigned integer
> comparisons" things
Sorry, I should have labelled this patch as "Try 2".
--
Andy.
Dmitry Timoshkov wrote:
> Andrew Talbot wrote:
>
>> Changelog:
>> gdi32: Indentation fix.
>
> Please keep 4 spaces indentation without tabs.
>
Thus far, I have fixed the bits of code that are misleadingly indented using
the same indentation regime as the sur
Perhaps I should add that the list is of caseS/defaultS that may be fallen
through to, rather than out from.
Unfortunately,because I produced it in a hurry, it does contain the odd
copy-and-paste error for case names (e.g., the case for dmusic/collection.c
line 409 should be default:, not case 8:,
Vincent Povirk wrote:
> It might be useful to post a listing of the files where unmarked
> fall-throughs (falls-through?) appear, and I could see if any of them
> are on my turf.
Here is a rough-and-ready list of where they are.
dlls/msvcp100/exception.c (line 498) case EXCEPTION:
dlls/msvcp100/
Jacek Caban wrote:
> It's probably better to change the macro to require the semicolon.
>
> Jacek
The reason I did it that way was because there are two variants of the
DEFINE_CXX_DATA macro, surrounded by an #ifndef construct: one comprising
three struct declarations, all ending in semicolons,
Frédéric Delanoy wrote:
> For every wine version, static checkers (like coverity) detect cases
> where a switch case automatically falls-through to the next case.
>
> Shouldn't be there a rule that such cases are always marked with a
> "fall-through comment"?
> With the possible exception of case
> Isn't it that way just to save the use of an extra return S_OK?
> Instead a break could be used too because that function returns S_OK
> by default.
>
> http://source.winehq.org/source/dlls/jscript/regexp.c#L4025
>
> Best regards,
> Bruno
A fall-through would indeed be lazy and not in the spir
Marcus Meissner wrote:
> Hi,
>
> If the filename and toolbar field are not present, we will be using
> uninitialized RECTs, so initialize them.
>
> CID 5033, 5034
>
> Ciao, Marcus
> ---
> dlls/comdlg32/itemdlg.c |2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/dl
Andrew Talbot wrote:
> Alex Bradbury wrote:
>
>> Marking fall through cases sounds reasonable on the face of it to me.
>> I question the necessity of adding 'unaudited' comments though. I'd
>> imagine lint or one of the more sophisticated static analysis
Alex Bradbury wrote:
> Marking fall through cases sounds reasonable on the face of it to me.
> I question the necessity of adding 'unaudited' comments though. I'd
> imagine lint or one of the more sophisticated static analysis tools
> could pretty easily give you a list of cases with fall-through
Hi All,
I am thinking of marking any unmarked places in switch statements where
fall-through occurs. However, simply to do so would be to ignore the
question of whether each fall-through is intentional or an oversight.
I therefore propose to mark each new point with two comments (maybe
separate,
Hi,
Static functions pe_get_sect() and pe_get_sect_size() in dbghelp/pe_module.c
are not called. May I remove them or does anyone have plans for them?
Thanks,
Andy.
Francois Gouget wrote:
> The bug is with -O3 which is not the default and which I would not
> personally care about. I'd be more open about maintenance issues.
>
My inclination would be to make the functions static. However, how to handle
the comments is an issue to consider. Also, I probably ma
Francois Gouget wrote:
>
> So dlls/rsaenh/mpi.c defines 13 functions that are only used there. So
> they could be made static by tweaking mpi.c and tomcrypt.h.
>
> However my understanding is that this files has been imported in Wine
> from another project so maybe it's not a good idea to diverg
Andrew Talbot wrote:
> The TRACE() potentially reporting the value of "newpath" must be placed
> after where that variable is first set.
>
I suspect that this patch still doesn't fix the problem. Feedback or expert
intervention is welcomed!
Thanks.
--
Andy.
Andrew Talbot wrote:
> It's a tabs vs spaces thing, but it looks way out on my system.
>
You might wish to ignore this patch. I had my tab stops set to four spaces
instead of eight, which exaggerated the distortion.
--
Andy.
Francois Gouget wrote:
> I could not find MapLS declared in winbase.h in any of the SDKs I have
> here. However I found it in win_me/inc16/thunks.h in an old DDK
> (Microsoft Windows 2000 DDK) but the declaration was:
>
> DWORD WINAPI MapLS(DWORD);
>
> But this being what looks like a 16bit in
Oops! Please ignore this one: I only changed the prototype of MapLS() and
forgot to change the definition.
--
Andy.
Have I got this right?
Thanks,
-- Andy.
---
Changelog:
winedos: Initializations fix.
diff --git a/dlls/winedos/int21.c b/dlls/winedos/int21.c
index 0c7967f..cbce913 100644
--- a/dlls/winedos/int21.c
+++ b/dlls/winedos/int21.c
@@ -2182,6 +2182,7 @@ static BOOL INT21_FileAttributes( CONTEXT86
Andrew Talbot wrote:
Sorry, title should be:
winedos: Replace [m|c]alloc() with HeapAlloc()
--
Andy.
Francois Gouget wrote:
> Also, for everyone's information, there's more calls to malloc() and
> free().
There are also many calls to realloc() - a function with complexities of its
own - and other functions with "realloc" in their name.
--
Andy.
Francois Gouget wrote:
>
> Thanks for the help with this task. With the last round of patches we
> are now down to about 280 warnings so there's definite progress. Here's
> the updated list:
>
> [...]
Here are the apparently unused functions I have encountered in the dlls so
far. Please speak u
Or "wineoss.drv:...", to be precise.
Francois Gouget wrote:
>
> I have attached a script that identifies functions that should be made
> static (among other things). There are approximately 450 of them, there
> should be pretty efw false positives, and I will look into them
> eventually. But if someone beats me to it I sure won't co
Jacek Caban wrote:
> I'm not fan of such fixes, but if you want to fix it, you should check
> len, not str, in your patch and you may move zero-terminating outside
> if..else statement.
>
>
> Jacek
Thank you, I shall fix it in the better way that you describe here.
--
Andy.
Dan Kegel wrote:
> I updated http://wiki.winehq.org/Wine64 with a list
> of some win64 apps. There are lots more than I
> expected.
Some of the top chess engines, such as Rybka (www.rybkachess.com) have
64-bit versions.
Jacek Caban wrote:
> The string was always zero-terminated without your patch. It's fine to
> call create_string with NULL str argument as long as len is 0 and
> current implementation works fine in this case.
>
>
> Jacek
Hi Jacek,
Technically, behavior is undefined if the pointers do not each
On Thu Dec 18 22:41 , 'James Hawkins' sent:
>I didn't write jscript, so I'm not the expert, but create_string is
>internal, so we should probably crash if str is NULL instead of hiding
>the error. What is this patch for?
>
>--
>James Hawkins
Hi James,
create_string() is called on line 1323
Christian Costa wrote:
> Hi Andrew,
>
> BTW, if the vtable are removed, there may be some unused functions. Will
> they be removed as well ?
>
> Bye,
> Christian
Hi Christian,
Because I was mindful not to remove such things lightly, that is why I
sought feedback from the community, and I shall
Ken Thomases wrote:
> On Dec 15, 2008, at 3:41 PM, Andrew Talbot wrote:
>
>> It appears that the following vtables and Wine debug channels are
>> not being used, so I am considering removing them. Please let me
>> know, therefore, if you have plans for any of
James Hawkins wrote:
>
> Why would you remove any of them? That's like removing stub functions
> because we don't know if they're ever called.
>
OK. I get the message; I shall leave the vtables alone. May I take out the
unused debug channels, though? I presume they can easily be re-introduced
It appears that the following vtables and Wine debug channels are not being
used, so I am considering removing them. Please let me know, therefore, if you
have plans for any of them and want them kept.
Vtables:
IDirectXFileBinary_Vtbl d3dxof/d3dxof.c
IDirectXFileObject_Vtbl
On Fri Dec 12 10:29 , Michael Stefaniuc sent:
>Andrew Talbot wrote:
>> But how would you then fix the sign-compare violation, or would you just let
>> this
>> one go?
>I had to look twice as the initial warning was in a for loop above:
>Either leave it as is for now
On Fri Dec 12 0:58 , Michael Stefaniuc sent:
>Andrew Talbot wrote:
>> What is wrong with this patch, please?
>If I may venture a guess: You have replaced a nice and concise for loop
>into and ugly 4 line while loop.
>
>bye
> michael
>
Hi Michael,
Ugly? An
Hi Juan,
Juan Lang wrote:
> The case I objected to is a curious one. I had a look at K&R's type
> promotion rules (2nd edition, section A6.5) and I'm confused what the
> compiler is doing here. The if-block is:
>
> if (pbEncoded[1] + 1 > cbEncoded)
>
> Rewriting the parenthesized expression a
Juan Lang wrote:
> Hi Andy,
>
> -if (pbEncoded[1] + 1 > cbEncoded)
> +if (pbEncoded[1] + 1U > cbEncoded)
>
> Is this change necessary? The resulting code is less clear than the
> original, IMO. It's clearly a spurious warning: a BYTE (max value
> 255) + 1 can't yield a value t
Please do not apply this patch, it is wrong.
--
Andy.
Austin English wrote:
>> This->baseShader.device;
>> int i;
>> -unsigned int extra_constants_needed = 0;
>> +unsigned int i, extra_constants_needed = 0;
>
> You forgot to remove 'int i' here.
>
Thanks, Austin. Good catch!
--
Andy.
James Hawkins wrote:
>> Sorry, I don't understand what I have done wrong. RegCloseKey() will
>> return ERROR_INVALID_HANDLE if called with hkey==NULL.
>>
>
> ...and we don't care what value it returns.
>
Ah, of course! Thanks, James (and Juan).
--
Andy.
James Hawkins wrote:
> On Tue, Sep 9, 2008 at 3:54 PM, Andrew Talbot
> <[EMAIL PROTECTED]> wrote:
>> Fix for Coverity error CID: 762.
>>
>> [...]
>> -RegCloseKey(userdata);
>> +if (userdata) RegCloseKey(userdata);
>> return rc;
>
Juan Lang wrote:
> Yes, I know what the value of CRYPT_E_NOT_FOUND is, and what the type
> of GetLastError is. My point is, Microsoft confused signed and
> unsigned types for their last error values. We have to live with it.
>
Indeed. (And sorry, I didn't mean to sound patronizing.)
> I don't
Juan Lang wrote:
> Hi Andy,
>
> +LONG last_error;
>
> ret = CryptGetObjectUrl(URL_OID_CERTIFICATE_CRL_DIST_POINT,
> rgpvContext[i], 0, NULL, &cbUrlArray, NULL, NULL, NULL);
> -if (!ret && GetLastError() == CRYPT_E_NOT_FOUND)
> +last_
Hi,
Is it possible to sneak in a bit of patch cleaning within the new software? It
would be useful to incidentally remove any trailing white space that happens to
exist within the scope of a patch.
--
Andy.
You might want to forget this one. I guess no one is compiling Wine on a
broken, pre-ANSI compiler. So the expansion of macro parameters inside
string literals may well, in effect, be a non-issue.
--
Andy.
James Hawkins wrote:
> On Sat, Aug 2, 2008 at 4:09 PM, Andrew Talbot
> <[EMAIL PROTECTED]> wrote:
>> Changelog:
>>fusion: Use proper function pointer.
>>
>> diff --git a/dlls/fusion/fusion.c b/dlls/fusion/fusion.c
>> index ac01cf4..637346c 100644
&
Andrew Talbot wrote:
> Andrew Talbot wrote:
>
>> (The reason I say
>> "decimal zero" is because decimal constants are signed, whereas
>> hexadecimal constants are unsigned[!] Thus, ~0x0 would be a viable
>> alternative.)
>>
>
> In fact, I hav
Andrew Talbot wrote:
> (The reason I say
> "decimal zero" is because decimal constants are signed, whereas
> hexadecimal constants are unsigned[!] Thus, ~0x0 would be a viable
> alternative.)
>
In fact, I have just tried both ~0 and ~0x0 and neither worked. (I can
Hi Michael,
Michael Stefaniuc wrote:
> Andrew Talbot wrote:
>> -if (!szConvertedList || dwFileCount == -1)
>> +if (!szConvertedList || (LONG)dwFileCount == -1)
> This one could be replaced by a comparison with either "-1u" or "~0".
>
Alexandre Julliard wrote:
> Static is for variables, not for types. Types are local to the file they
> are declared in, that's why you need header files when you want to share
> type declarations.
>
Ah, yes. It seems that only objects (i.e., named regions of storage) and
functions with external
Alexandre Julliard wrote:
> The types are local to the C file so there is no clash. If some tools
> don't understand that they need to be fixed.
>
Not arguing, just clarifying:
File #1: joystick_input.c has a non-static struct tag called "JoystickImpl".
File #2: joystick_linuxinput.c has a diff
Eric Pouech wrote:
> however the last trace should be protected (and debugstr_a is a better
> choice than testing for a null string)
> A+
>
Thanks, Eric. I've sent a replacement patch (with a revised title).
--
Andy.
Eric Pouech wrote:
> looks like a bit strange to me that you get a null typename here
> can you send me the DLL/.so file on which you get the seg fault
> A+
>
No known segfaults; I'm just doing static analysis. But
stabs_pts_read_type_def() is called several times within stabs.c passing
NULL as
Andrew Talbot wrote:
> If the forum is the wrong place to raise this sort of query, please
> forgive and advise. :)
>
Actually, Alexandre suggested that I file bug reports for things I find but
can't fix myself. And I suppose an indentation anomaly is still a sort of
bug(?)
--
Andy.
Hi,
In toolbar.c:TOOLBAR_Destroy(), should the "if" statement at line 5439 be
compound to match the indentation, or should the three invocations of
TOOLBAR_DeleteImageList() be outdented?
treeview.c:
In TREEVIEW_DeleteItem(), how conditional is the call of
TREEVIEW_SetFirstVisible() at line 15
Alistair Leslie-Hughes wrote:
> Hi Andrew,
> I would do 1, and if you think that its wrong, add a comment email
> asking
> for double check.
>
> Best Regards
> Alistair Leslie-Hughes
Sounds good. Thanks, Alistair!
--
Andy.
Hi All,
I'm intending to "correct" some indentation anomalies, and I propose to do
this in an "indiscriminate" way. In other words, I would set the
indentation to match what the code does now, without any interpretation of
what may have been intended. This means that something like
if (a)
James Hawkins wrote:
>
> These interface implementations could even be made static.
>
That's true. I shall still leave this patch in for consideration, since I
don't have time, right now, to properly consider what can be made static in
this dll. I hope to do so another time.
Thanks,
--
Andy.
I have marked this bug as fixed, because Alex has written some patches that,
indeed, fix it. But these will be held back until after the code freeze, so
the bug is still actually present in the current code. Would it be
preferable to reopen it, therefore, until the patches are actually applied?
Th
Jacek Caban wrote:
> Why? Both syntaxes are correct, so it's a matter of style preferences. I
> prefer the style I use and I don't see any reason to change it.
>
> Jacek
Speaking generally, there is one potential opportunity to create a
hard-to-find bug. If one has something like, say:
stru
Dan Kegel wrote:
> Andrew Talbot wrote:
>> witness my current
>> postings: "XBOOL, XBYTE, XINT8, etc." and "Five functions that cannot
>> handle a NULL parameter",
>
> I haven't seen those posts, where are they?
Hi Dan,
I posted them to w
Stefan Dösinger wrote:
> Am Montag, 5. Mai 2008 17:42:51 schrieb Andrew Talbot:
>> I have moved the TRACEs to where I think they belong. Please give
>> feedback if this patch is incorrect.
> on a quick look it looks OK. Did you check if any output is written in the
> case
Hi,
Frequently, I am finding minor bugs that I probably cannot fix myself, that
are probably not suitable for a Bugzilla bug report and that are likely to
be ignored if posted to wine-devel (witness my current postings: "XBOOL,
XBYTE, XINT8, etc." and "Five functions that cannot handle a NULL
para
In xlldrv.h, various scalar types are redefined, for example
#define BOOL X_BOOL
If these "X" variants exist on my system, they are not being included in
xfont.c. Where should I find them, please?
Thanks,
--
Andy.
In case anyone wishes to note or fix them, here are five functions that are
being called with a NULL parameter that they cannot yet properly handle,
and which they are passing down to a str...() or mem...() function. I may
be wrong, but I don't think these have been caught by Coverity. The number
i
Robert Shearman wrote:
> Again, this needs to be fixed in another way as fd is being leaked.
>
Thanks, again. I've sent a patch entitled
"dinput: Fix handle leak" to replace this.
--
Andy.
Robert Shearman wrote:
> A correct fix is to call CloseHandle(hThread), otherwise the handle is
> leaked.
>
Thanks, Rob. I've sent a replacement patch entitled
"browseui: Fix handle leak" to replace this.
--
Andy.
While we are on the subject of AVI files: could someone please take a look
at the function IAVIStream_fnWriteData() in avifil32/avifile.c? There is an
unused variable "dwPos" (line 1326), which has been there since this
function was first implemented (2002-10-18), I could just remove this
variable,
Robert Shearman wrote:
> It doesn't matter what MSDN says about szName, RegQueryValueExW still
> takes the size in bytes, not characters. I.e. count should be set to
> NAME_SIZE * sizeof(WCHAR), not NAME_SIZE.
>
Hi Robert,
Yes, indeed. I believe I may have had an editing accident with that one.
James Hawkins wrote:
>
> Don't you think a static const int would be even better?
>
Indeed. Re-submission imminent.
Thanks,
--
Andy.
John Klehm wrote:
If count needs to be the size of the buffer shouldn't it
> be:
>
> count = NAME_SIZE * sizeof(WCHAR);
>
> but probably better would be DMO_MAX_NAME_SIZE and be in a header
> somewhere (dmo.h?)?
>
> Regards,
> John
Ah yes, whoops. I'm pretty sure I had just that lined up, but
Robert Shearman wrote:
> This is incorrect. count is the size in bytes of the buffer passed in
> (szName) and so should be sizeof(szName) not
> sizeof(szName)/sizeof(szName[0]) (i.e. 80).
>
Are you sure? MSDN says "szName: Array of 80 Unicode characters that
receives the name of the DMO".
> If y
James Hawkins wrote:
> That's fine, but it's not worth it to me, and I'm pretty sure Julliard
> won't accept it either.
>
I understand and suspect you are right. Maybe I should have made an RFC
rather than opting for trial by patch. :)
Thanks for your comments.
--
Andy.
James Hawkins wrote:
>
> It's ugly. What warning are you trying to fix?
>
Although I imagine that gcc doesn't do anything particularly adverse as a
result of the existing code, if the pedantic switch were applied it would
cause a message of the following type to be generated.
action.c:236
James Hawkins wrote:
>
> I object. Also, RFCs should be sent to wine-devel, not wine-patches.
>
I was submitting a patch with a prelude explaining why, not making a request
for comment. But on what grounds are you objecting?
--
Andy.
Andrew Talbot wrote:
> strictly, when the static storage specifier is applied, the size must be
> specified:
>
> static char ar[5]; /* tentative definition */
>
> static char ar[5] = "hello"; /* actual definition */
>
Of course, the size need
Hi,
ws2_32/async.c has the following global declarations:
/* protoptypes of some functions in socket.c
*/
UINT16 wsaErrno(void);
UINT16 wsaHerrno(int errnr);
ws2_32/socket.c has the following global declarations:
UINT wsaErrno(void);
UINT wsaHerrno(int errnr);
Where are these functions define
Dan Kegel wrote:
> Yep. I filed http://bugs.winehq.org/show_bug.cgi?id=12098
> a while ago, it hits several apps. Please append your info there.
Done.
Stefan Dösinger wrote:
> Am Samstag, 22. März 2008 15:54:05 schrieb Andrew Talbot:
>> Hi Eric,
>>
>> An app I use - Blitzin2 - has a richedit control that used to wrap, and
>> since your patch "richedit: Added support for EM_SETTARGETDEVICE with a
>> NULL DC
Whoops! Sorry, I meant to send this to Eric [Pouech]. D'oh!
--
Andy.
Ken Thomases wrote:
> It's used in dlls/wintab32/wintab32.c via pGetCurrentPacket and you
> appear to have guessed correctly.
>
> Cheers,
> Ken
Looking good! Thanks for this, Ken.
--
Andy.
Robert Shearman wrote:
> This isn't correct. Judging by the surrounding code, this should be
> allocating a block of memory of This->pDesc->pbMemData and then passing
> pDesc->llMemLength into memcpy, possibly validating that
> pDesc->llMemLength isn't greater than UINT_MAX to avoid an overflow.
>
Stefan Dösinger wrote:
> Am Sonntag, 17. Februar 2008 01:38:50 schrieb Andrew Talbot:
>> And I presume that if the underlying struct tags
>> are different between two similar types, then the compiler would warn of
>> type incompatibility if such an assignment were attempted.
Stefan Dösinger wrote:
> Am Samstag, 16. Februar 2008 23:33:37 schrieb Andrew Talbot:
>> Here, I am assuming that the "dwSize" elements in all these cases should
>> be set to the size of the struct each is in, respectively. Please advise
>> if this assumption is wro
Vitaliy Margolen wrote:
> Marcus Meissner wrote:
>> On Sat, Feb 02, 2008 at 05:43:27PM +0000, Andrew Talbot wrote:
> Correct. What all that meant, is if app asks for something, but the value
> is
> 0, switch to default mode.
>
Yes. I wrongly mistook it to be a case of
Marcus Meissner wrote:
> I do not think this is right, def_mode is a boolean and we actually check
> for not-0-being of various struct members.
>
> Ciao, Marcus
Hi Marcus,
You are correct. I should have studied the code more carefully.
Thanks,
-- Andy.
By coincidence, I've edited the same line as Jacek Caban in his
patch "shdocvw: Store URL in BindStatusCallback" but in a different way. So
if the two clash, please drop my patch: I shall revise it and try again
after you have considered his.
Thanks,
--
Andy.
Hi Rico,
Please send your patches to wine-patches, not wine-devel.
Thanks,
--
Andy.
Please do not commit this patch.
Thanks,
--
Andy.
Please do not commit this patch.
Thanks,
--
Andy.
Please do not commit this patch. It relies on UINT (= unsigned int) and
DWORD (= unsigned long int) both being 32 bits wide.
Thanks,
--
Andy.
Michael Stefaniuc wrote:
> I just stumbled upon those while researching a potential problem for
> which Smatch issued a warning.
> [...]
> @@ -1324,7 +1324,7 @@ HDDEDATA WINAPI DdeAddData(HDDEDATA hData, LPBYTE
> pSrc, DWORD cb, DWORD cbOff)
> if (new_sz > old_sz)
> {
> DdeUnaccessData(
Samuel Lidén Borell wrote:
> Hi,
>
> I discovered a static constant wasn't initialized in RedrawWindow when I
> was using Valgrind. I wonder if this is a problem or not, because it has
> been there since 2005 and it doesn't give any compiler warnings. AFAIK
> constants can't be initialized later
[EMAIL PROTECTED] wrote:
>
Hi David,
There was no code in this patch.
--
Andy.
Please ignore this patch.
Thanks,
-- Andy.
Please ignore this patch: I overlooked the original scope of the
allocations.
--
Andy.
Paul Vriens wrote:
> Doesn't this mean that every one of those 4 mmio-calls are executed? In
> the previous logic we would bail out after one failure.
>
My reasoning is that short-circuit evaluation ensures that expressions are
evaluated from left to right, and as soon as one evaluates as true,
Kuba Ober wrote:
>> You calculating center wrong:
>> > + ret = (props->lMax-props->lMin)/2;
>>
>> This won't work for min=1000 max=2000.
>
> But it does. Maybe you meant if min/max were switched? In such case
>
> ret = (props->lMax-props->lMin)/2;
> if (props->lMax < props->lMin) ret = -ret;
>
Andrew Talbot wrote:
> So, in case anyone is still awake and interested:
>
> static const REFIID tid_ids[];
>
> is equivalent to
>
> static const GUID *const const tid_ids[]; (Note the erroneous double
> const.)
>
> or
> static GUID const *const
Michael Stefaniuc wrote:
> Jacek,
>
> I'm not sure about this patch. gcc 3.1.1 produced a warning about a
> double const. The rules of what is const depending on where in the
> declaration it is placed are mind bongling (especially at 1am) and I
> don't know what the intention was here so please
1 - 100 of 244 matches
Mail list logo
