From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1738 invoked by alias); 12 Dec 2019 17:18:59 -0000 Mailing-List: contact elfutils-devel-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Post: List-Help: List-Subscribe: Sender: elfutils-devel-owner@sourceware.org Received: (qmail 1713 invoked by uid 89); 12 Dec 2019 17:18:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.100.3 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL,BAYES_00,SPF_PASS,UNSUBSCRIBE_BODY autolearn=no version=3.3.1 spammy=Environment, dprintf, fche, manages X-Spam-Status: No, score=-2.7 required=5.0 tests=AWL,BAYES_00,SPF_PASS,UNSUBSCRIBE_BODY autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on sourceware.org X-Spam-Level: X-HELO: us-smtp-delivery-1.mimecast.com Received: from us-smtp-1.mimecast.com (HELO us-smtp-delivery-1.mimecast.com) (207.211.31.81) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 12 Dec 2019 17:18:57 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1576171136; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ioFsPcoZC0upCNZgUs8pJQpxeMlo3aknmYWFe/FdkNk=; b=KpuZ6ZhbdYkbh3TPyWcjt1ewnQWp9lbW/jkObwCbk3PWs0BUwi/EMeL4DuHLeW3UVYQopu 58jME9cgfQi1Gmmf+GSMjvcwYzO9MTJpGtSLinCgtcieR2EDwCpv9PB94DIi71/feGSIMt 2Y/WZFVupqkmetdiBQJ/cuVU1khj2Zo= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-339-0hsSLrt0NcaRYS5Vj1cL8g-1; Thu, 12 Dec 2019 12:18:52 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id AF1EA107ACC7; Thu, 12 Dec 2019 17:18:51 +0000 (UTC) Received: from redhat.com (ovpn-116-64.phx2.redhat.com [10.3.116.64]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 92FD367E40; Thu, 12 Dec 2019 17:18:51 +0000 (UTC) Received: from fche by redhat.com with local (Exim 4.92) (envelope-from ) id 1ifS70-0006yJ-3A; Thu, 12 Dec 2019 12:18:50 -0500 Date: Thu, 12 Dec 2019 17:18:00 -0000 From: "Frank Ch. Eigler" To: Mark Wielaard Cc: elfutils-devel@sourceware.org Subject: Re: rfc/patch: debuginfod client $DEBUGINFOD_PROGRESS env var Message-ID: <20191212171850.GE13089@redhat.com> References: <20191204211050.GA11981@redhat.com> <34b18e36d5a94512a27d8e1f65c9230e4803dc7d.camel@klomp.org> MIME-Version: 1.0 In-Reply-To: <34b18e36d5a94512a27d8e1f65c9230e4803dc7d.camel@klomp.org> User-Agent: Mutt/1.12.0 (2019-05-25) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-MC-Unique: 0hsSLrt0NcaRYS5Vj1cL8g-1 X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Content-Disposition: inline X-IsSubscribed: yes X-SW-Source: 2019-q4/txt/msg00270.txt.bz2 Hi - > > debuginfod: usability tweaks, incl. $DEBUGINFOD_PROGRESS client > > support > >=20=20=20=20=20 > > This facility allows a default progress-printing function > > to be installed if the given environment variable is set. >=20 > I like this idea but have a bit of a security concern about the ability > to set it to any file. If someone manages to set it then they can > overwrite anything a process using the library can write to.=20 That's not that serious a category of concern. Environment variables are not under control of untrusted agents. FWIW, $DEBUGINFOD_CACHE is considerably more dangerous in that regard (cache cleaning!). > I rather it would just use stderr always when set since it is only > meant as a human debugging device. That default makes sense. Making it configurable gives a way for a larger app to still capture output (by having the client direct it to a pipe file descriptor or something), but I'll just hardcode for now STDERR_FILENO if the env var is set. > > Some larger usage experience (systemtap/kernels) indicates > > the default timeout is too short, so bumped it to 30s. > My first thought is that we might need two timeouts. The original 5s > timeout is a good inactivity timeout [...] But for larger data we > might want a timeout for the total download time, maybe 30 seconds > is too low for that. Given how large kernel debuginfo is it might > take several minutes. OK, redrafting $DEBUGINFOD_TIMEOUT as a two-part variable: $DEBUGINFOD_TIMEOUT=3Dx,y x - connection timeout, default 5 y - optional, overall timeout, default unlimited > [...] > This seems to mean that if there is an error then the progress function > is never called, is that intended? Also for CURLM_CALL_MULTI_PERFORM we > continue/skip the progress function. If we have an error outcome, there is no more progress to report, so that should be OK. > > + close (fd); /* before the rmdir, otherwise it'll fail */ > > (void) rmdir (target_cache_dir); /* nop if not empty */ > > free(data); > > - close (fd); >=20 > This looks good. Just surprising. I assumed unlinking the file was > enough. Yeah, I had to see that for myself to believe it. > The \r \n trick is nice. > Just always using stderr would simplify this a bit. OK. > Also note that you can use dprintf to printf to a file descriptor.=20 OK. - FChE