[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#189866: awareness pong with patch



tag 189866 patch
thanks

Hi Jelle de Jong,
Hello everybody else reading this,

Prolog: I don't think the developers are not aware of this bug/wishlistitem
or need some (more) motivation. ;)

First of all, this is a minor thing because only a subset of the users use
the preference file and even less need comments in these files.
On the other hand, it is dangerous to play around with the TagFile Parser
because he is not only used for the minor important preference file:
This Parser is the backend to the dpkg status file (and APT extended status)
and therefore a really important part of APT.
Also, the status file is generated by dpkg in a parser-friendly way:
The parser can depend on the format, e.g. newlines separating a record and
so on and doesn't need to check for comments or other garbage
because dpkg doesn't write such garbage - so checks can be omitted
and the saved time can be used to do something useful instead.

The preference file on the other hand is user-generated.
A user want to write comments and doesn't want to follow a specific format
while writing his preference file - this is pretty the opposite of a well
formed status file. So we need (time consuming) checks here.

Emil's patch is therefore to unspecific:
It would also add this checks to the parsing of the status file
(and it doesn't ignore useless newlines or multiple line comments).

So after this explanation why a bit more specific work is needed
i will describe what my patch does:
It adds the virtual method TrimRecord which is called twice in each
pkgTagSection::Scan call. In it's standard implementation used by the status
files the first call does nothing and the second is an out sourced for loop
removing record-separating newlines. This is exactly what is done without
the patch so nothing is changed beside the (little) overhead of two virtual
method calls.
In the police.cc the pkgTagSection::TrimRecord method is overridden
which removes all newlines and comments before (first call)
and after (second call) the record.

So you can use/write comments and newlines n-times before, between and
after records, but you can't use comments in between a record and
a comment directly after a record. You see, it is not perfect, but a bit
better than the status quo and doesn't need a rewrite of the parser...

Any comments?
(apart from that my english is "under all pig" and that i wrote to much ;)

Sidenote: In the merged bug #374654 is commenting in apt.conf with #
requested. Commenting in the config files is already implemented with
C-like comments ( // and /* */ ), but i support the request for allowing
also the # as a comment marker and the patch for this is a one-liner...

So the attached patch does the following:
* add the # -comment to the preference file as described above
* allow also the #-comment in the config files
( * fix some foldmethod markers )


Best regards / Mit freundlichen Grüßen,

David "DonKult" Kalnischkies
=== modified file 'apt-pkg/contrib/configuration.cc'
--- apt-pkg/contrib/configuration.cc	2009-05-14 13:21:27 +0000
+++ apt-pkg/contrib/configuration.cc	2009-06-05 20:34:48 +0000
@@ -595,7 +595,7 @@
 	 if (InQuote == true)
 	    continue;
 	 
-	 if (*I == '/' && I + 1 != End && I[1] == '/')
+	 if ((*I == '/' && I + 1 != End && I[1] == '/') || *I == '#')
          {
 	    End = I;
 	    break;

=== modified file 'apt-pkg/policy.cc'
--- apt-pkg/policy.cc	2007-06-08 23:44:03 +0000
+++ apt-pkg/policy.cc	2009-06-05 19:07:01 +0000
@@ -239,7 +239,21 @@
    return 0;
 }
 									/*}}}*/
-
+// PreferenceSection class - Overriding the default TrimRecord method	/*{{{*/
+// ---------------------------------------------------------------------
+/* The preference file is a user generated file so the parser should
+   therefore be a bit more friendly by allowing comments and new lines
+   all over the place rather than forcing a special format */
+class PreferenceSection : public pkgTagSection
+{
+   void TrimRecord(bool BeforeRecord, const char* &End)
+   {
+      for (; Stop < End && (Stop[0] == '\n' || Stop[0] == '\r' || Stop[0] == '#'); Stop++)
+	 if (Stop[0] == '#')
+	    Stop = (const char*) memchr(Stop,'\n',End-Stop);
+   }
+};
+									/*}}}*/
 // ReadPinFile - Load the pin file into a Policy			/*{{{*/
 // ---------------------------------------------------------------------
 /* I'd like to see the preferences file store more than just pin information
@@ -259,7 +273,7 @@
    if (_error->PendingError() == true)
       return false;
    
-   pkgTagSection Tags;
+   PreferenceSection Tags;
    while (TF.Step(Tags) == true)
    {
       string Name = Tags.FindS("Package");

=== modified file 'apt-pkg/tagfile.cc'
--- apt-pkg/tagfile.cc	2007-06-08 23:44:03 +0000
+++ apt-pkg/tagfile.cc	2009-06-05 19:22:39 +0000
@@ -81,7 +81,7 @@
    End = Start + EndSize;
    return true;
 }
-
+									/*}}}*/
 // TagFile::Step - Advance to the next section				/*{{{*/
 // ---------------------------------------------------------------------
 /* If the Section Scanner fails we refill the buffer and try again. 
@@ -212,10 +212,12 @@
 
    if (Stop == 0)
       return false;
-   
+
    TagCount = 0;
    while (TagCount+1 < sizeof(Indexes)/sizeof(Indexes[0]) && Stop < End)
    {
+       TrimRecord(true,End);
+
       // Start a new index and add it to the hash
       if (isspace(Stop[0]) == 0)
       {
@@ -227,14 +229,14 @@
       
       if (Stop == 0)
 	 return false;
-      
+
       for (; Stop+1 < End && Stop[1] == '\r'; Stop++);
 
       // Double newline marks the end of the record
       if (Stop+1 < End && Stop[1] == '\n')
       {
 	 Indexes[TagCount] = Stop - Section;
-	 for (; Stop < End && (Stop[0] == '\n' || Stop[0] == '\r'); Stop++);
+	 TrimRecord(false,End);
 	 return true;
       }
       
@@ -244,6 +246,16 @@
    return false;
 }
 									/*}}}*/
+// TagSection::TrimRecord - Trim off any garbage before/after a record	/*{{{*/
+// ---------------------------------------------------------------------
+/* There should be exactly 2 newline at the end of the record, no more. */
+void pkgTagSection::TrimRecord(bool BeforeRecord, const char*& End)
+{
+   if (BeforeRecord == true)
+      return;
+   for (; Stop < End && (Stop[0] == '\n' || Stop[0] == '\r'); Stop++);
+}
+									/*}}}*/
 // TagSection::Trim - Trim off any trailing garbage			/*{{{*/
 // ---------------------------------------------------------------------
 /* There should be exactly 1 newline at the end of the buffer, no more. */
@@ -390,7 +402,6 @@
    return true;
 }
 									/*}}}*/
-
 // TFRewrite - Rewrite a control record					/*{{{*/
 // ---------------------------------------------------------------------
 /* This writes the control record to stdout rewriting it as necessary. The

=== modified file 'apt-pkg/tagfile.h'
--- apt-pkg/tagfile.h	2007-07-26 17:18:11 +0000
+++ apt-pkg/tagfile.h	2009-06-05 17:53:05 +0000
@@ -27,7 +27,6 @@
 class pkgTagSection
 {
    const char *Section;
-   const char *Stop;
    
    // We have a limit of 256 tags per section.
    unsigned int Indexes[256];
@@ -35,6 +34,9 @@
    
    unsigned int TagCount;
      
+   protected:
+   const char *Stop;
+
    public:
    
    inline bool operator ==(const pkgTagSection &rhs) {return Section == rhs.Section;};
@@ -49,6 +51,7 @@
    bool Scan(const char *Start,unsigned long MaxLength);
    inline unsigned long size() const {return Stop - Section;};
    void Trim();
+   virtual void TrimRecord(bool BeforeRecord, const char* &End);
    
    inline unsigned int Count() const {return TagCount;};
    inline void Get(const char *&Start,const char *&Stop,unsigned int I) const


Reply to: