From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9807 invoked by alias); 20 Jun 2012 19:36:30 -0000 Received: (qmail 9625 invoked by uid 22791); 20 Jun 2012 19:36:29 -0000 X-SWARE-Spam-Status: No, hits=-4.0 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_DNSWL_NONE,RCVD_IN_HOSTKARMA_NO,SPF_SOFTFAIL X-Spam-Check-By: sourceware.org Received: from mtaout20.012.net.il (HELO mtaout20.012.net.il) (80.179.55.166) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 20 Jun 2012 19:36:14 +0000 Received: from conversion-daemon.a-mtaout20.012.net.il by a-mtaout20.012.net.il (HyperSendmail v2007.08) id <0M5X00800L3MEL00@a-mtaout20.012.net.il> for gdb-patches@sourceware.org; Wed, 20 Jun 2012 22:36:01 +0300 (IDT) Received: from HOME-C4E4A596F7 ([87.69.210.75]) by a-mtaout20.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0M5X007PML40VMC0@a-mtaout20.012.net.il>; Wed, 20 Jun 2012 22:36:00 +0300 (IDT) Date: Wed, 20 Jun 2012 19:36:00 -0000 From: Eli Zaretskii Subject: Re: [patch 1/2] auto-load safe-path: Permit shell wildcards In-reply-to: <20120620184032.GA13928@host2.jankratochvil.net> To: Jan Kratochvil Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: <83txy5dbdd.fsf@gnu.org> References: <20120617193703.GA5088@host2.jankratochvil.net> <83ehpceh5h.fsf@gnu.org> <20120620184032.GA13928@host2.jankratochvil.net> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-06/txt/msg00643.txt.bz2 > Date: Wed, 20 Jun 2012 20:40:32 +0200 > From: Jan Kratochvil > Cc: gdb-patches@sourceware.org > > > > -filename_is_in_dir (const char *filename, const char *dir) > > > +filename_is_in_pattern (const char *filename_orig, const char *pattern_orig) > > > > The arguments are named differently from what the commentary says. > > (Do you really need the "_orig" suffix here?) > > I have split it now into filename_is_in_pattern and filename_is_in_pattern_1. > > As an explanation of the previous state: > > The problem is that I want to use bare "filename" in the code. Using > "filename_local" (for example) may lead to mistakes as it is easier to > accidentally write "filename" than to accidentally write "filename_orig". > > Using just "filename" everywhere would mean to make the parameter non-const > ('char *filename') which may lead callers into thinking the string contents > may be modified by this function. The code both uses and modifies the content > of "filename". So you wanted to use 'const', which dominoed into a whole bunch of variable renaming and helper function. Looks like a net loss to me. But I won't argue about style, since I'm usually in the minority on that. > > > + /* Trim trailing slashes ("/") from PATTERN. */ > > > + while (pattern_len && IS_DIR_SEPARATOR (pattern[pattern_len - 1])) > > > + pattern_len--; > > > + pattern[pattern_len] = '\0'; > > > > Wouldn't this will do the wrong thing with a pattern such as "d:/"? > > It will trim it to "d:" and then it will try to remove /+[^/]+ each time from > the filename reducing it also down to "d:". Therefore "d:/" will match any > file on drive d:. I'd say that warrants a comment, since the code looks plain wrong. > > > @value{GDBN} provides the @samp{set auto-load safe-path} setting to list > > > directories trusted for loading files not explicitly requested by user. > > > +Each directory can be also shell wildcard pattern. > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > "... can also be a shell wildcard." > > I believe there should still be "shell wildcard pattern": > > set auto-load safe-path patternX:patternY > set auto-load safe-path /directoryX1/directoryX2:/directoryY1/directoryY2 > > I can say: > set auto-load safe-path /usr/src*/.gdbinit > > So even a part of the directory ("src*") can be shell wildcard ("*"). > "src*" is IMO 'shell wildcard pattern' and not 'shell wildcard'. > "*" is 'shell wildcard'. I believe you are talking about "wildcard character" as opposed to a "wildcard". > Do you still insist on your wording? I don't insist, but I still think "pattern" is redundant. > > > + '*' matches only single > > > +component, it does not match across directory separator. > > > > "... @samp{*} matches a single component ..." > > > > Should we describe all of the wildcard meta-characters, not just '*'? > > The goal was to express FNM_FILE_NAME is in use. I understand, but I'm not sure the readers will. It sounds strange to mention only one wildcard character, unless you already know that FNM_FILE_NAME is the issue, and you also know what it does.