Re: memmem issues
On Dec 19 21:01, Eric Blake wrote: Index: libc/memmem.cc === RCS file: /cvs/src/src/winsup/cygwin/libc/memmem.cc,v retrieving revision 1.1 diff -u -p -r1.1 memmem.cc --- libc/memmem.cc8 Nov 2005 22:08:39 - 1.1 +++ libc/memmem.cc20 Dec 2007 03:56:35 - @@ -45,8 +45,8 @@ memmem (const void *l, size_t l_len, const char *cs = (const char *)s; /* we need something to compare */ - if (l_len == 0 || s_len == 0) -return NULL; + if (s_len == 0) +return l; Looks like this is actually more correct. Glibc and NetBSD both agree with you, while only the FreeBSD function seems to return NULL in this case. However, it's not quite ok. l is const void * while the function returns void *. I applied this part of your patch with the obvious cast. + /* FIXME - this algorithm is worst-case O(l_len*s_len), but using + Knuth-Morris-Pratt would be O(l_len+s_len) at the expense of a + memory allocation of s_len bytes. Consider rewriting this to + swap over the KMP if the first few iterations fail, and back to + this if KMP can't allocate enough memory. */ for (cur = (char *) cl; cur = last; cur++) if (cur[0] == cs[0] memcmp (cur, cs, s_len) == 0) return cur; What about just using documented example code, for instance from here: http://www-igm.univ-mlv.fr/~lecroq/string/node8.html or here: http://de.wikipedia.org/wiki/Knuth-Morris-Pratt-Algorithmus (in German) or what about Boyer-Moore instead: http://de.wikipedia.org/wiki/Boyer-Moore-Algorithmus (in German) Using one of them is certainly not a licensing violation since all code examples are more or less the published examples from well-known textbooks (Knuth, Sedgewick, et al.). Given that, I don't think you're actually tainted. An actual implementation would be much better than a forlorn comment in an unimpressive file in some subdirectory. Thanks, Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Project Co-Leader cygwin AT cygwin DOT com Red Hat
[patch] un-NT-ify cygcheck (was: cygwin 1.5.25-7: cygcheck does not work?)
Brian Dessent wrote:
Fortunately, I have VMware with a Win98 image here.
The problem is that bloda.c calls NtQuerySystemInformation without using
any kind of autoload.cc-type indirection, and so cygcheck gets a hard
dependency on ntdll.dll which doesn't exist on 9x/ME. We need to do one
of:
- Revert the bloda-check feature on the 1.5 branch
- Check windows version at runtime and only do NT calls through
LoadLibrary/GetProcAddress
- Use the autoload.cc trick in cygcheck
If we're going to make releases from the 1.5 branch then I don't think
it's quite acceptible just yet to shaft 9x users, after all that's the
whole point of the branch.
The attached patch un-NT-ifies bloda.cc but sadly a similar cleanup is
still required for cygpath as well if we are to support 9x/ME out of the
1.5 branch. In that case I suppose you could just revert cygpath.cc to
an older revision before the native APIs were added.
Brian2007-12-20 Brian Dessent [EMAIL PROTECTED]
* Makefile.in (cygcheck.exe): Don't link to ntdll.
* bloda.cc (pNtQuerySystemInformation): Add.
(pRtlAnsiStringToUnicodeString): Add.
(get_process_list): Use function pointers for NT functions.
(dump_dodgy_apps): Skip dodgy app check on non-NT platforms.
Use GetProcAddress for NT-specific functions.
Index: Makefile.in
===
RCS file: /cvs/src/src/winsup/utils/Makefile.in,v
retrieving revision 1.67
diff -u -p -r1.67 Makefile.in
--- Makefile.in 3 Aug 2007 19:41:48 - 1.67
+++ Makefile.in 20 Dec 2007 15:08:23 -
@@ -104,10 +104,10 @@ ifeq $(libz)
@echo '*** Building cygcheck without package content checking due to
missing mingw libz.a.'
endif
ifdef VERBOSE
- $(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,4,$^} -B$(mingw_build)/
$(MINGW_LDFLAGS) $(libz) -lntdll
+ $(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,4,$^} -B$(mingw_build)/
$(MINGW_LDFLAGS) $(libz)
else
- @echo $(CXX) -o $@ ${wordlist 1,4,$^} ${filter-out -B%,
$(MINGW_CXXFLAGS) $(MINGW_LDFLAGS)} $(libz) -lntdll;\
- $(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,4,$^} -B$(mingw_build)/
$(MINGW_LDFLAGS) $(libz) -lntdll
+ @echo $(CXX) -o $@ ${wordlist 1,4,$^} ${filter-out -B%,
$(MINGW_CXXFLAGS) $(MINGW_LDFLAGS)} $(libz);\
+ $(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,4,$^} -B$(mingw_build)/
$(MINGW_LDFLAGS) $(libz)
endif
dumper.o: dumper.cc dumper.h
Index: bloda.cc
===
RCS file: /cvs/src/src/winsup/utils/bloda.cc,v
retrieving revision 1.1
diff -u -p -r1.1 bloda.cc
--- bloda.cc3 Aug 2007 19:41:48 - 1.1
+++ bloda.cc20 Dec 2007 15:08:23 -
@@ -104,13 +104,20 @@ static const size_t num_of_dodgy_apps =
to be looked up at runtime and called through a pointer. */
VOID NTAPI (*pRtlFreeUnicodeString)(PUNICODE_STRING) = NULL;
+NTSTATUS NTAPI (*pNtQuerySystemInformation) (SYSTEM_INFORMATION_CLASS,
+ PVOID, ULONG, PULONG) = NULL;
+
+NTSTATUS NTAPI (*pRtlAnsiStringToUnicodeString) (PUNICODE_STRING, PANSI_STRING,
+ BOOLEAN) = NULL;
+
+
static PSYSTEM_PROCESSES
get_process_list (void)
{
int n_procs = 0x100;
PSYSTEM_PROCESSES pslist = (PSYSTEM_PROCESSES) malloc (n_procs * sizeof
*pslist);
- while (NtQuerySystemInformation (SystemProcessesAndThreadsInformation,
+ while (pNtQuerySystemInformation (SystemProcessesAndThreadsInformation,
pslist, n_procs * sizeof *pslist, 0) == STATUS_INFO_LENGTH_MISMATCH)
{
n_procs *= 2;
@@ -126,7 +133,7 @@ get_module_list (void)
int modsize = 0x1000;
PSYSTEM_MODULE_INFORMATION modlist = (PSYSTEM_MODULE_INFORMATION) malloc
(modsize);
- while (NtQuerySystemInformation (SystemModuleInformation,
+ while (pNtQuerySystemInformation (SystemModuleInformation,
modlist, modsize, NULL) == STATUS_INFO_LENGTH_MISMATCH)
{
modsize *= 2;
@@ -284,19 +291,14 @@ detect_dodgy_app (const struct bad_app_d
/* Equivalent of RtlInitAnsiString. */
ansiname.Length = ansiname.MaximumLength = strlen (det-param);
ansiname.Buffer = (CHAR *) det-param;
- rv = RtlAnsiStringToUnicodeString (unicodename, ansiname, TRUE);
+ rv = pRtlAnsiStringToUnicodeString (unicodename, ansiname, TRUE);
if (rv != STATUS_SUCCESS)
{
printf (Ansi to unicode conversion failure $%08x\n, (unsigned int)
rv);
break;
}
found = find_process_in_list (pslist, unicodename);
- if (!pRtlFreeUnicodeString)
- pRtlFreeUnicodeString = (VOID NTAPI (*)(PUNICODE_STRING))
GetProcAddress (LoadLibrary (ntdll.dll), RtlFreeUnicodeString);
- if (pRtlFreeUnicodeString)
-pRtlFreeUnicodeString (unicodename);
- else
-printf (leaking mem...oops\n);
+ pRtlFreeUnicodeString (unicodename);
if (found)
{
Re: [patch] un-NT-ify cygcheck (was: cygwin 1.5.25-7: cygcheck does not work?)
Brian Dessent wrote: ... but sadly a similar cleanup is still required for cygpath as well if we are to support 9x/ME out of the 1.5 branch. In that case I suppose you could just revert cygpath.cc to an older revision before the native APIs were added. Er, nevermind. I was accidently looking at HEAD, but the native stuff in cygpath is not on the branch. So I think only the bloda.cc change is necessary to restore 9x/ME capability on the branch. Brian
Re: [patch] un-NT-ify cygcheck (was: cygwin 1.5.25-7: cygcheck does not work?)
On Thu, Dec 20, 2007 at 07:15:53AM -0800, Brian Dessent wrote: Brian Dessent wrote: Fortunately, I have VMware with a Win98 image here. The problem is that bloda.c calls NtQuerySystemInformation without using any kind of autoload.cc-type indirection, and so cygcheck gets a hard dependency on ntdll.dll which doesn't exist on 9x/ME. We need to do one of: - Revert the bloda-check feature on the 1.5 branch - Check windows version at runtime and only do NT calls through LoadLibrary/GetProcAddress - Use the autoload.cc trick in cygcheck If we're going to make releases from the 1.5 branch then I don't think it's quite acceptible just yet to shaft 9x users, after all that's the whole point of the branch. The attached patch un-NT-ifies bloda.cc but sadly a similar cleanup is still required for cygpath as well if we are to support 9x/ME out of the 1.5 branch. In that case I suppose you could just revert cygpath.cc to an older revision before the native APIs were added. Brian 2007-12-20 Brian Dessent [EMAIL PROTECTED] * Makefile.in (cygcheck.exe): Don't link to ntdll. * bloda.cc (pNtQuerySystemInformation): Add. (pRtlAnsiStringToUnicodeString): Add. (get_process_list): Use function pointers for NT functions. (dump_dodgy_apps): Skip dodgy app check on non-NT platforms. Use GetProcAddress for NT-specific functions. I had something similar in my sandbox but 1) you beat me to it and 2) yours is better. So, please check this into the trunk. I don't have any problem with cygcheck being Windows 9x aware since it could help us track down problems with people who are trying to run Cygwin 1.7.x on older systems. Unless Corinna says differently, I think she wants to be in control of what goes into the branch so I don't want to suggest that you should check it in there too. cgf
RE: [patch] un-NT-ify cygcheck (was: cygwin 1.5.25-7: cygcheck does not work?)
On 20 December 2007 21:12, Christopher Faylor wrote: On Thu, Dec 20, 2007 at 07:15:53AM -0800, Brian Dessent wrote: The attached patch un-NT-ifies bloda.cc but sadly a similar cleanup is still required for cygpath as well if we are to support 9x/ME out of the 1.5 branch. In that case I suppose you could just revert cygpath.cc to an older revision before the native APIs were added. I had something similar in my sandbox but 1) you beat me to it and 2) yours is better. So, please check this into the trunk. I don't have any problem with cygcheck being Windows 9x aware since it could help us track down problems with people who are trying to run Cygwin 1.7.x on older systems. Unless Corinna says differently, I think she wants to be in control of what goes into the branch so I don't want to suggest that you should check it in there too. But it only belongs on the branch at all, doesn't it? When I wrote bloda.cc, I didn't bother with 9x compat, because I was only writing it for HEAD, where we have stopped supporting 9x. Surely there are many other reasons why HEAD won't work on 9x, so the only benefit would be in applying this patch to the branch? cheers, DaveK -- Can't think of a witty .sigline today
Re: [patch] un-NT-ify cygcheck (was: cygwin 1.5.25-7: cygcheck does not work?)
On Fri, Dec 21, 2007 at 01:59:54AM -, Dave Korn wrote: On 20 December 2007 21:12, Christopher Faylor wrote: On Thu, Dec 20, 2007 at 07:15:53AM -0800, Brian Dessent wrote: The attached patch un-NT-ifies bloda.cc but sadly a similar cleanup is still required for cygpath as well if we are to support 9x/ME out of the 1.5 branch. In that case I suppose you could just revert cygpath.cc to an older revision before the native APIs were added. I had something similar in my sandbox but 1) you beat me to it and 2) yours is better. So, please check this into the trunk. I don't have any problem with cygcheck being Windows 9x aware since it could help us track down problems with people who are trying to run Cygwin 1.7.x on older systems. Unless Corinna says differently, I think she wants to be in control of what goes into the branch so I don't want to suggest that you should check it in there too. But it only belongs on the branch at all, doesn't it? When I wrote bloda.cc, I didn't bother with 9x compat, because I was only writing it for HEAD, where we have stopped supporting 9x. Surely there are many other reasons why HEAD won't work on 9x, so the only benefit would be in applying this patch to the branch? I explained my logic above: I don't have any problem with cygcheck being Windows 9x aware since it could help us track down problems with people who are trying to run Cygwin 1.7.x on older systems. The problem in this case would be Hey! Look at what cygcheck is saying! You are using Windows 9x! You can't do that! OTOH, you could make a case that cygcheck on the trunk should just consider Windows 9x and friends to be dodgy apps and should issue a clear error in that case. cgf
Re: [patch] un-NT-ify cygcheck (was: cygwin 1.5.25-7: cygcheck does not work?)
Christopher Faylor wrote:
Unless Corinna says differently, I think she wants to be in control of
what goes into the branch so I don't want to suggest that you should
check it in there too.
Okay, I'll let her take care of the branch since she's been handling all
the releases from it.
The problem in this case would be Hey! Look at what cygcheck is saying!
You are using Windows 9x! You can't do that!
In a sense it already does this:
case VER_PLATFORM_WIN32_WINDOWS:
switch (osversion.dwMinorVersion)
{
case 0:
osname = 95 (not supported);
break;
case 10:
osname = 98 (not supported);
break;
case 90:
osname = ME (not supported);
break;
default:
osname = 9X (not supported);
break;
}
break;
Brian
