Re: [d-i] Yet Another bug with pre-sprintf malloc:ing

2002-08-30 Thread Tollef Fog Heen

* Martin Sjögren 

| ons 2002-08-28 klockan 16.45 skrev Tollef Fog Heen:
| > * Colin Walters 
| > 
| > | I guess this is up to Mithrandir...what do you think?
| > 
| > I'd say go ahead.
| 
| I attached a patch for using asprintf in anna, main-menu, choose-mirror,
| wget-retriever and udpkg. udpkg's makefile.in also adds -D_GNU_SOURCE.
| Not checking it in configure is suboptimal, yes, but I can't summon up
| enough courage to work with autotools right now.

thanks.  all applied.

-- 
Tollef Fog Heen,''`.
UNIX is user friendly, it's just picky about who its friends are  : :' :
  `. `' 
`-  


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]




Re: [d-i] Yet Another bug with pre-sprintf malloc:ing

2002-08-29 Thread Nathan Hawkins

Michael Goetze wrote:
>>(Just recording what we said on IRC)
>>So far -bsd haven't even properly decided on what libc to use.  We
>>need to have a lot more working Debian/*BSD before we think about
>>writing an installer for *BSD.
>>
>>Still, keeping the portability at maximum would be good.
>>
>>(-bsd Cc-ed, so they can voice their opinions, if any.)
> 
> I don't know what needs to happen for any decision about Debian/*BSD to be
> considered "proper", but the de facto status right now seems to be that it's
> the BSD libc, since ports of the GNU libc are either experimental or
> nonexistent...

The FreeBSD port of glibc is working. Most Debian sources I've tried 
build and work perfectly. There are two basic issues:

1. Thread support isn't finished
2. Massive work needed on kernel headers

---Nathan


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]




Re: [d-i] Yet Another bug with pre-sprintf malloc:ing

2002-08-29 Thread Michael Goetze


> (Just recording what we said on IRC)
> 
> So far -bsd haven't even properly decided on what libc to use.  We
> need to have a lot more working Debian/*BSD before we think about
> writing an installer for *BSD.
> 
> Still, keeping the portability at maximum would be good.
> 
> (-bsd Cc-ed, so they can voice their opinions, if any.)

I don't know what needs to happen for any decision about Debian/*BSD to be
considered "proper", but the de facto status right now seems to be that it's
the BSD libc, since ports of the GNU libc are either experimental or
nonexistent...

- Michael

=
-BEGIN GEEK CODE BLOCK-
Version: 3.12
GCS/MU d->+ s++:+ a--- C++ ULISBH+++ P+++ L++(-) E-
W--(+) N+ o? K? w--- !O !M V?> PS+++ !PE Y+ PGP-
t+(-) 5 X R+>+++ tv-- b++>+ DI(+) D G e>+++ h r-- y
--END GEEK CODE BLOCK--

__
Do You Yahoo!?
Yahoo! Finance - Get real-time stock quotes
http://finance.yahoo.com


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]




Re: [d-i] Yet Another bug with pre-sprintf malloc:ing

2002-08-29 Thread Tollef Fog Heen

* Martin Sjögren 

| ons 2002-08-28 klockan 16.45 skrev Tollef Fog Heen:
| > * Colin Walters 
| > 
| > | I guess this is up to Mithrandir...what do you think?
| > 
| > I'd say go ahead.
| 
| Are there any implications for debian/freebsd, or should we take them
| when we get there? asprintf, as it so happens, exists on *bsd too, but
| if we start using _GNU_SOURCE all over we might have problems later.

(Just recording what we said on IRC)

So far -bsd haven't even properly decided on what libc to use.  We
need to have a lot more working Debian/*BSD before we think about
writing an installer for *BSD.

Still, keeping the portability at maximum would be good.

(-bsd Cc-ed, so they can voice their opinions, if any.)

-- 
Tollef Fog Heen,''`.
UNIX is user friendly, it's just picky about who its friends are  : :' :
  `. `' 
`-  


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]




Re: [d-i] Yet Another bug with pre-sprintf malloc:ing

2002-08-28 Thread Martin Sjögren

ons 2002-08-28 klockan 16.45 skrev Tollef Fog Heen:
> * Colin Walters 
> 
> | I guess this is up to Mithrandir...what do you think?
> 
> I'd say go ahead.

I attached a patch for using asprintf in anna, main-menu, choose-mirror,
wget-retriever and udpkg. udpkg's makefile.in also adds -D_GNU_SOURCE.
Not checking it in configure is suboptimal, yes, but I can't summon up
enough courage to work with autotools right now.


Regards,
Martin


Index: anna/anna.c
===
RCS file: /cvs/debian-boot/debian-installer/anna/anna.c,v
retrieving revision 1.12
diff -u -r1.12 anna.c
--- anna/anna.c	27 May 2002 14:08:45 -	1.12
+++ anna/anna.c	28 Aug 2002 16:53:15 -
@@ -46,9 +46,9 @@
 
 /* Calls udpkg to unpack a package. */
 int unpack_package (char *pkgfile) {
-	char *command=malloc(strlen(DPKG_UNPACK_COMMAND) + 1 +
-			 strlen(pkgfile) + 1);
-	sprintf(command, "%s %s", DPKG_UNPACK_COMMAND, pkgfile);
+	char *command;
+
+	asprintf(&command, "%s %s", DPKG_UNPACK_COMMAND, pkgfile);
 	return ! system(command);
 }
 
@@ -98,9 +98,7 @@
 			for(f = fp = p->filename; *fp != 0; fp++)
 if (*fp == '/')
 	f = ++fp;
-			dest_file=malloc(strlen(DOWNLOAD_DIR) + 1 +
-	 strlen(f) + 1);
-			sprintf(dest_file, "%s/%s", DOWNLOAD_DIR, f);
+			asprintf(&dest_file, "%s/%s", DOWNLOAD_DIR, f);
 
 			if (! get_package(p, dest_file)) {
 fprintf(stderr, "anna: error getting %s!\n",
Index: anna/retriever.c
===
RCS file: /cvs/debian-boot/debian-installer/anna/retriever.c,v
retrieving revision 1.12
diff -u -r1.12 retriever.c
--- anna/retriever.c	28 Aug 2002 07:24:49 -	1.12
+++ anna/retriever.c	28 Aug 2002 16:53:15 -
@@ -81,9 +81,9 @@
 int get_package (struct package_t *package, char *dest) {
 	int ret;
 	char *retriever=chosen_retriever();
-	char *command=malloc(strlen(retriever) + 1 + strlen(package->filename) +
- 1 + strlen(dest) + 1);
-	sprintf(command, "%s %s %s", retriever, package->filename, dest);
+	char *command;
+
+	asprintf(&command, "%s %s %s", retriever, package->filename, dest);
 	ret=! system(command);
 	free(command);
 	return ret;
@@ -108,11 +108,10 @@
 int currsuite = 0;
 suite = suites[currsuite];
 while (suite != NULL) {
-  char *command=malloc(strlen(retriever) + 10 + 
-   sizeof(tmp_packages) + 1 + strlen(suite));
+		char *command;
 
 unlink(tmp_packages);
-sprintf(command, "%s Packages %s %s", retriever, tmp_packages, 
+asprintf(&command, "%s Packages %s %s", retriever, tmp_packages, 
 suite);
 fprintf(stderr,"%s\n", command);
 if (system(command) != 0) {
Index: main-menu/main-menu.c
===
RCS file: /cvs/debian-boot/debian-installer/main-menu/main-menu.c,v
retrieving revision 1.30
diff -u -r1.30 main-menu.c
--- main-menu/main-menu.c	27 May 2002 14:12:56 -	1.30
+++ main-menu/main-menu.c	28 Aug 2002 16:53:16 -
@@ -85,16 +85,21 @@
 
 /* Returns true if the given package could be the default menu item. */
 int isdefault(struct package_t *p) {
-	char menutest[1024];
+	char *menutest;
 	struct stat statbuf;
+	int ret;
 
-	sprintf(menutest, DPKGDIR "info/%s.menutest", p->package);
+	asprintf(&menutest, DPKGDIR "info/%s.menutest", p->package);
 	if (stat(menutest, &statbuf) == 0) {
-		return ! SYSTEM(menutest);
+		ret = SYSTEM(menutest);
+		free(menutest);
+		return !ret;
 	}
 	else if (p->status == unpacked || p->status == half_configured) {
+		free(menutest);
 		return 1;
 	}
+	free(menutest);
 	return 0;
 }
 
@@ -206,13 +211,16 @@
 }
 
 int do_menu_item(struct package_t *p) {
-	char configcommand[1024];
+	char *configcommand;
 	struct package_t *head = NULL, *tail = NULL;
+	int ret;
 	
 	if (p->status == installed) {
 		/* The menu item is already configured, so reconfigure it. */
-		sprintf(configcommand, "dpkg-reconfigure %s", p->package);
-		return ! SYSTEM(configcommand);
+		asprintf(&configcommand, "dpkg-reconfigure %s", p->package);
+		ret = SYSTEM(configcommand);
+		free(configcommand);
+		return !ret;
 	}
 	else if (p->status == unpacked || p->status == half_configured) {
 		/*
@@ -223,8 +231,10 @@
 		order_done(head);
 		for (p = head; p; p = p->next) {
 			if (p->status == unpacked || p->status == half_configured) {
-sprintf(configcommand, DPKG_CONFIGURE_COMMAND " %s", p->package);
-if (SYSTEM(configcommand) != 0)
+asprintf(&configcommand, DPKG_CONFIGURE_COMMAND " %s", p->package);
+ret = SYSTEM(configcommand);
+free(configcommand);
+if (ret != 0)
 	return 0; /* give up on failure */
 			}
 		}
Index: retriever/choose-mirror/choose-mirror.c
===
RCS file: /cvs/debian-boot/debian-installer/retriever/choose-mirror/choose-mirror.c,v
retrieving rev

Re: [d-i] Yet Another bug with pre-sprintf malloc:ing

2002-08-28 Thread Martin Sjögren

ons 2002-08-28 klockan 16.45 skrev Tollef Fog Heen:
> * Colin Walters 
> 
> | I guess this is up to Mithrandir...what do you think?
> 
> I'd say go ahead.

Are there any implications for debian/freebsd, or should we take them
when we get there? asprintf, as it so happens, exists on *bsd too, but
if we start using _GNU_SOURCE all over we might have problems later.


Regards,
Martin



signature.asc
Description: Detta =?ISO-8859-1?Q?=E4r?= en digitalt signeradmeddelandedel


Re: [d-i] Yet Another bug with pre-sprintf malloc:ing

2002-08-28 Thread Tollef Fog Heen

* Colin Walters 

| I guess this is up to Mithrandir...what do you think?

I'd say go ahead.

-- 
Tollef Fog Heen,''`.
UNIX is user friendly, it's just picky about who its friends are  : :' :
  `. `' 
`-  


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]




Re: [d-i] Yet Another bug with pre-sprintf malloc:ing

2002-08-28 Thread Martin Sjögren

ons 2002-08-28 klockan 09.51 skrev Martin Sjögren:
> ons 2002-08-28 klockan 09.34 skrev Colin Walters:
> > On Mon, 2002-08-26 at 06:51, Martin Sjögren wrote:
> > > I found Yet Another bug with "pre-sprintf" malloc:ing in anna (in
> > > retriever.c, get_packages), a patch is attached.
> > 
> > This kind of thing is exactly why GNU came up with the "asprintf"
> > extension.  Since we're using glibc and gcc, I suggest we have a policy
> > of allowing GNU extensions, such as glibc library functions, and GNU C.
> 
> That's what I realized too while blubbering about with glib's
> g_strdup_printf. I don't see how it would harm to use GNU extensions.

I mucked about a bit. Turns out that anna, main-menu, choose-mirror and
wget-retriever are already compiled with -D_GNU_SOURCE. udpkg, however,
is not. I don't really see much of a reason for anyone to use udpkg
without "the rest" of d-i, which already use GNU extensions. I don't
think this is my call though. :)


Regards,
Martin



signature.asc
Description: Detta =?ISO-8859-1?Q?=E4r?= en digitalt signeradmeddelandedel


Re: [d-i] Yet Another bug with pre-sprintf malloc:ing

2002-08-28 Thread Martin Sjögren

ons 2002-08-28 klockan 09.34 skrev Colin Walters:
> On Mon, 2002-08-26 at 06:51, Martin Sjögren wrote:
> > I found Yet Another bug with "pre-sprintf" malloc:ing in anna (in
> > retriever.c, get_packages), a patch is attached.
> 
> This kind of thing is exactly why GNU came up with the "asprintf"
> extension.  Since we're using glibc and gcc, I suggest we have a policy
> of allowing GNU extensions, such as glibc library functions, and GNU C.

That's what I realized too while blubbering about with glib's
g_strdup_printf. I don't see how it would harm to use GNU extensions.


Martin



signature.asc
Description: Detta =?ISO-8859-1?Q?=E4r?= en digitalt signeradmeddelandedel


Re: [d-i] Yet Another bug with pre-sprintf malloc:ing

2002-08-28 Thread Colin Walters

On Mon, 2002-08-26 at 06:51, Martin Sjögren wrote:
> I found Yet Another bug with "pre-sprintf" malloc:ing in anna (in
> retriever.c, get_packages), a patch is attached.

This kind of thing is exactly why GNU came up with the "asprintf"
extension.  Since we're using glibc and gcc, I suggest we have a policy
of allowing GNU extensions, such as glibc library functions, and GNU C.

I guess this is up to Mithrandir...what do you think?


--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]




Re: [d-i] Yet Another bug with pre-sprintf malloc:ing

2002-08-27 Thread Tollef Fog Heen

* Martin Sjögren 

| I found Yet Another bug with "pre-sprintf" malloc:ing in anna (in
| retriever.c, get_packages), a patch is attached.

thanks, applied.  Will try to remember to check in when I get online
again.. :)

| This got me thinking that we could avoid these kind of bugs if we had
| some sort of decent string lib. I think the glib string functions are
| pretty neat and my suggestion is to rip out the ones we need and put in
| libd-i, perhaps under the "namespace" di_str_. Specificially,
| g_strdup_sprintf would solve exactly these kind of problems for us.

can you try to do this, build the library and see how big it gets?  If
it's not a too big size increase, I say we go for it.

| Since Debian uses text files a lot, there's a lot of string juggling,
| and that's no fun to do in C unless you've got decent tools...

true.

-- 
Tollef Fog Heen,''`.
UNIX is user friendly, it's just picky about who its friends are  : :' :
  `. `' 
`-  


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]




[d-i] Yet Another bug with pre-sprintf malloc:ing

2002-08-26 Thread Martin Sjögren

I found Yet Another bug with "pre-sprintf" malloc:ing in anna (in
retriever.c, get_packages), a patch is attached.

This got me thinking that we could avoid these kind of bugs if we had
some sort of decent string lib. I think the glib string functions are
pretty neat and my suggestion is to rip out the ones we need and put in
libd-i, perhaps under the "namespace" di_str_. Specificially,
g_strdup_sprintf would solve exactly these kind of problems for us.

Since Debian uses text files a lot, there's a lot of string juggling,
and that's no fun to do in C unless you've got decent tools...


Regards,
Martin


Index: retriever.c
===
RCS file: /cvs/debian-boot/debian-installer/anna/retriever.c,v
retrieving revision 1.11
diff -u -r1.11 retriever.c
--- retriever.c	25 Aug 2002 20:14:09 -	1.11
+++ retriever.c	26 Aug 2002 10:51:36 -
@@ -47,7 +47,8 @@
 suite = suites[currsuite];
 while (suite != NULL) {
   char *command=malloc(strlen(retriever) + 10 + 
-   sizeof(tmp_packages) + 1);
+   sizeof(tmp_packages) + 1 +
+   strlen(suite) + 1);
 
 unlink(tmp_packages);
 sprintf(command, "%s Packages %s %s", retriever, tmp_packages, 



signature.asc
Description: Detta =?ISO-8859-1?Q?=E4r?= en digitalt signeradmeddelandedel