larmbr zhan wrote:
On Sun, Mar 10, 2013 at 5:48 AM, Andrew Talbot
andrew.tal...@talbotville.com wrote:
msvcp60: Avoid signed-unsigned integer comparisons.
Hi, Andrew Talbot.
I find that you are working on these Avoid signed-unsigned integer
comparisons things recently.
I DO agree
Dmitry Timoshkov wrote:
Andrew Talbot andrew.tal...@talbotville.com 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 surrounding
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 with
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,
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:
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
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 spirit of
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 tools
could pretty easily give you a list
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
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,
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,
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:
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 diverge
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 made
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.
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 include
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.
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 up for
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
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:
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.
On Thu Dec 18 22:41 , 'James Hawkins' trui...@gmail.com 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
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
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
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
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
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 them and want them kept.
Wine debug
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? Andrew Koenig and Barbara Moo show
On Fri Dec 12 10:29 , Michael Stefaniuc mstef...@redhat.com 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 as the warning
Hi Juan,
Juan Lang wrote:
The case I objected to is a curious one. I had a look at KR'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 as types
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 that
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:
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;
Please don't add another NULL-before-free check.
Hi James,
Sorry
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.
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_error =
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
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.
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
--- a/dlls/fusion/fusion.c
+++ b/dlls/fusion/fusion.c
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.
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.
bye
michael
The first of these would work; the second
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't figure
out why the latter fails
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 have just tried both ~0 and ~0x0 and neither worked. (I can't
figure
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
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
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 a
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.
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
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 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)
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.
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.
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:
struct
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
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 of failures in those functions
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 wine-devel on Saturday.
I'd say a conformance test would
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
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.
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.
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.
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.
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 you
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 I reckon
James Hawkins wrote:
Don't you think a static const int would be even better?
Indeed. Re-submission imminent.
Thanks,
--
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 not be specified in the case
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.
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:
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.
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
Whoops! Sorry, I meant to send this to Eric [Pouech]. D'oh!
--
Andy.
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
Wrong mailing list?
Wrong address entirely. My
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.
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.
Yes.
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.
Not quite. You can have
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 wrong.
It's not necessarily true
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.
Vitaliy Margolen wrote:
Marcus Meissner wrote:
On Sat, Feb 02, 2008 at 05:43:27PM +, 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 setting one group of flags
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. It relies on UINT (= unsigned int) and
DWORD (= unsigned long int) both being 32 bits wide.
Thanks,
--
Andy.
Please do not commit this patch.
Thanks,
--
Andy.
Please do not commit this patch.
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(hData);
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;
Cheers, Kuba
If
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
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 const tid_ids[];
as I would prefer to put
Yes. It seems that the corresponding MSDN web page is out of step.
--
Andy.
Misha Koshelev wrote:
On a more stylistic note, any reason for all these single line if
statements that have their contents enclosed in {}'s? (i.e., instead of:
if (a) {
call_b();
}
why not just:
if (a)
call_b();
If anything, this adds one unnecessary line with the single }
1 - 100 of 233 matches
Mail list logo
